From 5e29740560e6345c498807d3882dd100d5848ca2 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Wed, 16 Oct 2024 14:26:18 -0400 Subject: [PATCH 1/8] feature: implementation of xdg_activation_v1 because I want my auto focuses! --- src/server/frontend_wayland/CMakeLists.txt | 1 + .../frontend_wayland/wayland_connector.cpp | 6 +- .../frontend_wayland/wayland_connector.h | 5 +- .../wayland_default_configuration.cpp | 8 +- .../frontend_wayland/xdg_activation_v1.cpp | 312 ++++++++++++++++++ .../frontend_wayland/xdg_activation_v1.h | 45 +++ src/wayland/CMakeLists.txt | 1 + wayland-protocols/xdg-activation-v1.xml | 200 +++++++++++ 8 files changed, 574 insertions(+), 4 deletions(-) create mode 100644 src/server/frontend_wayland/xdg_activation_v1.cpp create mode 100644 src/server/frontend_wayland/xdg_activation_v1.h create mode 100644 wayland-protocols/xdg-activation-v1.xml diff --git a/src/server/frontend_wayland/CMakeLists.txt b/src/server/frontend_wayland/CMakeLists.txt index 1d0e325594a..7997af9dab3 100644 --- a/src/server/frontend_wayland/CMakeLists.txt +++ b/src/server/frontend_wayland/CMakeLists.txt @@ -65,6 +65,7 @@ set( ${PROJECT_SOURCE_DIR}/src/include/server/mir/frontend/buffer_stream.h wp_viewporter.cpp wp_viewporter.h fractional_scale_v1.cpp fractional_scale_v1.h + xdg_activation_v1.cpp xdg_activation_v1.h ) add_custom_command( diff --git a/src/server/frontend_wayland/wayland_connector.cpp b/src/server/frontend_wayland/wayland_connector.cpp index f6486d70b0f..632809bd625 100644 --- a/src/server/frontend_wayland/wayland_connector.cpp +++ b/src/server/frontend_wayland/wayland_connector.cpp @@ -241,7 +241,8 @@ mf::WaylandConnector::WaylandConnector( WaylandProtocolExtensionFilter const& extension_filter, bool enable_key_repeat, std::shared_ptr const& session_lock, - std::shared_ptr const& decoration_strategy) + std::shared_ptr const& decoration_strategy, + std::shared_ptr const& focus_controller) : extension_filter{extension_filter}, display{wl_display_create(), &cleanup_display}, pause_signal{eventfd(0, EFD_CLOEXEC | EFD_SEMAPHORE)}, @@ -328,7 +329,8 @@ mf::WaylandConnector::WaylandConnector( main_loop, desktop_file_manager, session_lock_, - decoration_strategy}); + decoration_strategy, + focus_controller}); shm_global = std::make_unique(display.get(), executor); diff --git a/src/server/frontend_wayland/wayland_connector.h b/src/server/frontend_wayland/wayland_connector.h index 096cc52eb28..9ab92a53385 100644 --- a/src/server/frontend_wayland/wayland_connector.h +++ b/src/server/frontend_wayland/wayland_connector.h @@ -59,6 +59,7 @@ class DisplayConfigurationObserver; namespace shell { class Shell; +class FocusController; } namespace scene { @@ -113,6 +114,7 @@ class WaylandExtensions std::shared_ptr desktop_file_manager; std::shared_ptr session_lock; std::shared_ptr decoration_strategy; + std::shared_ptr focus_controller; }; WaylandExtensions() = default; @@ -165,7 +167,8 @@ class WaylandConnector : public Connector WaylandProtocolExtensionFilter const& extension_filter, bool enable_key_repeat, std::shared_ptr const& session_lock, - std::shared_ptr const& decoration_strategy); + std::shared_ptr const& decoration_strategy, + std::shared_ptr const& focus_controller); ~WaylandConnector() override; diff --git a/src/server/frontend_wayland/wayland_default_configuration.cpp b/src/server/frontend_wayland/wayland_default_configuration.cpp index 9e9b66dab4b..c4bd0ff0c63 100644 --- a/src/server/frontend_wayland/wayland_default_configuration.cpp +++ b/src/server/frontend_wayland/wayland_default_configuration.cpp @@ -43,6 +43,7 @@ #include "wl_seat.h" #include "wl_shell.h" #include "wlr_screencopy_v1.h" +#include "xdg_activation_v1.h" #include "xdg-decoration-unstable-v1_wrapper.h" #include "xdg_decoration_unstable_v1.h" #include "xdg_output_v1.h" @@ -226,6 +227,10 @@ std::vector const internal_extension_builders = { { return mf::create_fractional_scale_v1(ctx.display); }), + make_extension_builder([](auto const& ctx) + { + return mf::create_xdg_activation_v1(ctx.display, ctx.shell, ctx.focus_controller, ctx.main_loop); + }), }; ExtensionBuilder const xwayland_builder { @@ -384,7 +389,8 @@ std::shared_ptr wayland_extension_filter, enable_repeat, the_session_lock(), - the_decoration_strategy()); + the_decoration_strategy(), + the_focus_controller()); }); } diff --git a/src/server/frontend_wayland/xdg_activation_v1.cpp b/src/server/frontend_wayland/xdg_activation_v1.cpp new file mode 100644 index 00000000000..7c1f8d8b608 --- /dev/null +++ b/src/server/frontend_wayland/xdg_activation_v1.cpp @@ -0,0 +1,312 @@ +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "xdg_activation_v1.h" +#include "wl_surface.h" +#include "wl_seat.h" +#include "mir/main_loop.h" +#include "mir/scene/surface.h" +#include "mir/shell/shell.h" +#include "mir/shell/focus_controller.h" +#include "mir/log.h" +#include +#include + +namespace mf = mir::frontend; +namespace mw = mir::wayland; +namespace msh = mir::shell; + +namespace mir::frontend +{ +/// Time in milliseconds that the compositor will wait before invalidating a token +auto constexpr TIMEOUT_MS = std::chrono::milliseconds(3000); + +std::string generate_token() +{ + std::string str("0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"); + std::random_device rd; + std::mt19937 generator(rd()); + std::shuffle(str.begin(), str.end(), generator); + return str.substr(0, 32); +} + +class XdgActivationV1 : public wayland::XdgActivationV1::Global +{ +public: + XdgActivationV1( + struct wl_display* display, + std::shared_ptr const& shell, + std::shared_ptr const& focus_controller, + std::shared_ptr const& main_loop); + + std::string const& create_token(); + bool try_consume_token(std::string const& token); + +private: + class Instance : public wayland::XdgActivationV1 + { + public: + Instance( + struct wl_resource* resource, + mf::XdgActivationV1* xdg_activation_v1, + std::shared_ptr const& shell, + std::shared_ptr const& focus_controller, + std::shared_ptr const& main_loop); + + private: + void get_activation_token(struct wl_resource* id) override; + void activate(std::string const& token, struct wl_resource* surface) override; + + mf::XdgActivationV1* xdg_activation_v1; + std::shared_ptr shell; + std::shared_ptr const focus_controller; + std::shared_ptr main_loop; + }; + + struct Token + { + std::string token; + std::unique_ptr alarm; + }; + + void bind(wl_resource* resource) override; + + std::shared_ptr shell; + std::shared_ptr const focus_controller; + std::shared_ptr main_loop; + std::vector pending_tokens; + std::mutex lock; +}; + +class XdgActivationTokenV1 : public wayland::XdgActivationTokenV1 +{ +public: + XdgActivationTokenV1( + struct wl_resource* resource, + std::string const& token, + std::shared_ptr const& focus_controller); + +private: + void set_serial(uint32_t serial, struct wl_resource* seat) override; + void set_app_id(std::string const& app_id) override; + void set_surface(struct wl_resource* surface) override; + void commit() override; + + std::string token; + std::shared_ptr const focus_controller; + std::optional serial; + std::optional seat; + std::optional app_id; + std::optional requesting_surface; +}; +} + +auto mf::create_xdg_activation_v1( + struct wl_display* display, + std::shared_ptr const& shell, + std::shared_ptr const& focus_controller, + std::shared_ptr const& main_loop) + -> std::shared_ptr +{ + return std::make_shared(display, shell, focus_controller, main_loop); +} + +mf::XdgActivationV1::XdgActivationV1( + struct wl_display* display, + std::shared_ptr const& shell, + std::shared_ptr const& focus_controller, + std::shared_ptr const& main_loop) + : Global(display, Version<1>()), + shell{shell}, + focus_controller{focus_controller}, + main_loop{main_loop} +{ + +} + +std::string const& mf::XdgActivationV1::create_token() +{ + auto generated = generate_token(); + Token token{generated, main_loop->create_alarm([this, generated]() + { + try_consume_token(generated); + })}; + token.alarm->reschedule_in(TIMEOUT_MS); + + std::lock_guard guard(lock); + pending_tokens.emplace_back(std::move(token)); + return pending_tokens[pending_tokens.size() - 1].token; +} + +bool mf::XdgActivationV1::try_consume_token(std::string const& token) +{ + std::lock_guard guard(lock); + for (auto it = pending_tokens.begin(); it != pending_tokens.end(); it++) + { + if (it->token == token) + { + pending_tokens.erase(it); + return true; + } + } + + return false; +} + +void mf::XdgActivationV1::bind(struct wl_resource* resource) +{ + new Instance{resource, this, shell, focus_controller, main_loop}; +} + +mf::XdgActivationV1::Instance::Instance( + struct wl_resource* resource, + mf::XdgActivationV1* xdg_activation_v1, + std::shared_ptr const& shell, + std::shared_ptr const& focus_controller, + std::shared_ptr const& main_loop) + : XdgActivationV1(resource, Version<1>()), + xdg_activation_v1{xdg_activation_v1}, + shell{shell}, + focus_controller{focus_controller}, + main_loop{main_loop} +{ +} + +void mf::XdgActivationV1::Instance::get_activation_token(struct wl_resource* id) +{ + new XdgActivationTokenV1(id, xdg_activation_v1->create_token(), focus_controller); +} + +void mf::XdgActivationV1::Instance::activate(std::string const& token, struct wl_resource* surface) +{ + if (!xdg_activation_v1->try_consume_token(token)) + { + mir::log_error("XdgActivationV1::activate invalid token: %s", token.c_str()); + return; + } + + main_loop->enqueue(this, [surface=surface, shell=shell] + { + auto const wl_surface = mf::WlSurface::from(surface); + if (!wl_surface) + { + mir::log_error("XdgActivationV1::activate wl_surface not found"); + return; + } + + auto const scene_surface_opt = wl_surface->scene_surface(); + if (!scene_surface_opt) + { + mir::log_error("XdgActivationV1::activate scene_surface_opt not found"); + return; + } + + auto const& scene_surface = scene_surface_opt.value(); + auto const now = std::chrono::steady_clock::now().time_since_epoch(); + auto ns = std::chrono::duration_cast(now).count(); + shell->raise_surface(wl_surface->session, scene_surface, ns); + }); +} + +mf::XdgActivationTokenV1::XdgActivationTokenV1( + struct wl_resource* resource, + std::string const& token, + std::shared_ptr const& focus_controller) + : wayland::XdgActivationTokenV1(resource, Version<1>()), + token{token}, + focus_controller{focus_controller} +{ +} + +void mf::XdgActivationTokenV1::set_serial(uint32_t serial_, struct wl_resource* seat_) +{ + serial = serial_; + seat = seat_; +} + +void mf::XdgActivationTokenV1::set_app_id(std::string const& app_id_) +{ + app_id = app_id_; +} + +void mf::XdgActivationTokenV1::set_surface(struct wl_resource* surface) +{ + requesting_surface = surface; +} + +void mf::XdgActivationTokenV1::commit() +{ + // In the event of a failure, we send 'done' with a new, invalid token + // which cannot be used later. + + if (seat && serial) + { + // First, assert that the seat still exists. + auto const wl_seat = mf::WlSeat::from(seat.value()); + if (!wl_seat) + { + mir::log_error("XdgActivationTokenV1::commit wl_seat not found"); + send_done_event(generate_token()); + return; + } + + // TODO: + // Next, verify that the serial belongs to the seat + } + + if (app_id) + { + // Check that the focused application has app_id + auto const& focused = focus_controller->focused_surface(); + if (!focused) + { + mir::log_error("XdgActivationTokenV1::commit cannot find the focused surface"); + send_done_event(generate_token()); + return; + } + + if (focused->application_id() != app_id) + { + mir::log_error("XdgActivationTokenV1::commit app_id != focused application id"); + send_done_event(generate_token()); + return; + } + } + + if (requesting_surface) + { + // Check that the focused surface is the requesting_surface + auto const wl_surface = mf::WlSurface::from(requesting_surface.value()); + auto const scene_surface_opt = wl_surface->scene_surface(); + if (!scene_surface_opt) + { + mir::log_error("XdgActivationTokenV1::commit cannot find the provided scene surface"); + send_done_event(generate_token()); + return; + } + + auto const& scene_surface = scene_surface_opt.value(); + + if (focus_controller->focused_surface() != scene_surface) + { + mir::log_error("XdgActivationTokenV1::commit the focused surface is not the surface that requesteda ctivation"); + send_done_event(generate_token()); + return; + } + } + + send_done_event(token); +} diff --git a/src/server/frontend_wayland/xdg_activation_v1.h b/src/server/frontend_wayland/xdg_activation_v1.h new file mode 100644 index 00000000000..b6dd5587199 --- /dev/null +++ b/src/server/frontend_wayland/xdg_activation_v1.h @@ -0,0 +1,45 @@ +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef MIR_FRONTEND_XDG_ACTIVATION_UNSTABLE_V1_H +#define MIR_FRONTEND_XDG_ACTIVATION_UNSTABLE_V1_H + +#include "xdg-activation-v1_wrapper.h" + +struct wl_display; + +namespace mir +{ +class MainLoop; +namespace shell +{ +class Shell; +class FocusController; +} + +namespace frontend +{ +auto create_xdg_activation_v1( + struct wl_display* display, + std::shared_ptr const&, + std::shared_ptr const&, + std::shared_ptr const&) -> + std::shared_ptr; +} +} + + +#endif //MIR_FRONTEND_XDG_ACTIVATION_UNSTABLE_V1_H diff --git a/src/wayland/CMakeLists.txt b/src/wayland/CMakeLists.txt index 33e9f4b9118..e4820524fab 100644 --- a/src/wayland/CMakeLists.txt +++ b/src/wayland/CMakeLists.txt @@ -39,6 +39,7 @@ mir_generate_protocol_wrapper(mirwayland "zmir_" mir-shell-unstable-v1.xml) mir_generate_protocol_wrapper(mirwayland "z" xdg-decoration-unstable-v1.xml) mir_generate_protocol_wrapper(mirwayland "wp_" viewporter.xml) mir_generate_protocol_wrapper(mirwayland "wp_" fractional-scale-v1.xml) +mir_generate_protocol_wrapper(mirwayland "z" xdg-activation-v1.xml) target_link_libraries(mirwayland PUBLIC diff --git a/wayland-protocols/xdg-activation-v1.xml b/wayland-protocols/xdg-activation-v1.xml new file mode 100644 index 00000000000..5c3c4fed299 --- /dev/null +++ b/wayland-protocols/xdg-activation-v1.xml @@ -0,0 +1,200 @@ + + + + + Copyright © 2020 Aleix Pol Gonzalez <aleixpol@kde.org> + Copyright © 2020 Carlos Garnacho <carlosg@gnome.org> + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice (including the next + paragraph) shall be included in all copies or substantial portions of the + Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. + + + + The way for a client to pass focus to another toplevel is as follows. + + The client that intends to activate another toplevel uses the + xdg_activation_v1.get_activation_token request to get an activation token. + This token is then forwarded to the client, which is supposed to activate + one of its surfaces, through a separate band of communication. + + One established way of doing this is through the XDG_ACTIVATION_TOKEN + environment variable of a newly launched child process. The child process + should unset the environment variable again right after reading it out in + order to avoid propagating it to other child processes. + + Another established way exists for Applications implementing the D-Bus + interface org.freedesktop.Application, which should get their token under + activation-token on their platform_data. + + In general activation tokens may be transferred across clients through + means not described in this protocol. + + The client to be activated will then pass the token + it received to the xdg_activation_v1.activate request. The compositor can + then use this token to decide how to react to the activation request. + + The token the activating client gets may be ineffective either already at + the time it receives it, for example if it was not focused, for focus + stealing prevention. The activating client will have no way to discover + the validity of the token, and may still forward it to the to be activated + client. + + The created activation token may optionally get information attached to it + that can be used by the compositor to identify the application that we + intend to activate. This can for example be used to display a visual hint + about what application is being started. + + Warning! The protocol described in this file is currently in the testing + phase. Backward compatible changes may be added together with the + corresponding interface version bump. Backward incompatible changes can + only be done by creating a new major version of the extension. + + + + + A global interface used for informing the compositor about applications + being activated or started, or for applications to request to be + activated. + + + + + Notify the compositor that the xdg_activation object will no longer be + used. + + The child objects created via this interface are unaffected and should + be destroyed separately. + + + + + + Creates an xdg_activation_token_v1 object that will provide + the initiating client with a unique token for this activation. This + token should be offered to the clients to be activated. + + + + + + + + Requests surface activation. It's up to the compositor to display + this information as desired, for example by placing the surface above + the rest. + + The compositor may know who requested this by checking the activation + token and might decide not to follow through with the activation if it's + considered unwanted. + + Compositors can ignore unknown activation tokens when an invalid + token is passed. + + + + + + + + + An object for setting up a token and receiving a token handle that can + be passed as an activation token to another client. + + The object is created using the xdg_activation_v1.get_activation_token + request. This object should then be populated with the app_id, surface + and serial information and committed. The compositor shall then issue a + done event with the token. In case the request's parameters are invalid, + the compositor will provide an invalid token. + + + + + + + + + Provides information about the seat and serial event that requested the + token. + + The serial can come from an input or focus event. For instance, if a + click triggers the launch of a third-party client, the launcher client + should send a set_serial request with the serial and seat from the + wl_pointer.button event. + + Some compositors might refuse to activate toplevels when the token + doesn't have a valid and recent enough event serial. + + Must be sent before commit. This information is optional. + + + + + + + + The requesting client can specify an app_id to associate the token + being created with it. + + Must be sent before commit. This information is optional. + + + + + + + This request sets the surface requesting the activation. Note, this is + different from the surface that will be activated. + + Some compositors might refuse to activate toplevels when the token + doesn't have a requesting surface. + + Must be sent before commit. This information is optional. + + + + + + + Requests an activation token based on the different parameters that + have been offered through set_serial, set_surface and set_app_id. + + + + + + The 'done' event contains the unique token of this activation request + and notifies that the provider is done. + + + + + + + Notify the compositor that the xdg_activation_token_v1 object will no + longer be used. The received token stays valid. + + + + From 51f798f0f7682eb55e779ca38af5388eee6fda13 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 17 Oct 2024 08:39:36 -0400 Subject: [PATCH 2/8] feedback: minor PR nits + using uuid instead of generating a unique string by ourselves --- src/server/frontend_wayland/CMakeLists.txt | 1 + .../frontend_wayland/xdg_activation_v1.cpp | 28 +++++++++++++------ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/server/frontend_wayland/CMakeLists.txt b/src/server/frontend_wayland/CMakeLists.txt index 7997af9dab3..954da2b643b 100644 --- a/src/server/frontend_wayland/CMakeLists.txt +++ b/src/server/frontend_wayland/CMakeLists.txt @@ -114,5 +114,6 @@ target_link_libraries(mirfrontend-wayland mircore PRIVATE PkgConfig::GIOUnix + PkgConfig::UUID ) diff --git a/src/server/frontend_wayland/xdg_activation_v1.cpp b/src/server/frontend_wayland/xdg_activation_v1.cpp index 7c1f8d8b608..90fca581bbe 100644 --- a/src/server/frontend_wayland/xdg_activation_v1.cpp +++ b/src/server/frontend_wayland/xdg_activation_v1.cpp @@ -24,25 +24,29 @@ #include "mir/log.h" #include #include +#include namespace mf = mir::frontend; namespace mw = mir::wayland; namespace msh = mir::shell; -namespace mir::frontend +namespace { /// Time in milliseconds that the compositor will wait before invalidating a token -auto constexpr TIMEOUT_MS = std::chrono::milliseconds(3000); +auto constexpr timeout_ms = std::chrono::milliseconds(3000); std::string generate_token() { - std::string str("0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"); - std::random_device rd; - std::mt19937 generator(rd()); - std::shuffle(str.begin(), str.end(), generator); - return str.substr(0, 32); + uuid_t uuid; + uuid_generate(uuid); + return { reinterpret_cast(uuid), 4 }; +} } +namespace mir +{ +namespace frontend +{ class XdgActivationV1 : public wayland::XdgActivationV1::Global { public: @@ -53,6 +57,7 @@ class XdgActivationV1 : public wayland::XdgActivationV1::Global std::shared_ptr const& main_loop); std::string const& create_token(); + bool try_consume_token(std::string const& token); private: @@ -68,6 +73,7 @@ class XdgActivationV1 : public wayland::XdgActivationV1::Global private: void get_activation_token(struct wl_resource* id) override; + void activate(std::string const& token, struct wl_resource* surface) override; mf::XdgActivationV1* xdg_activation_v1; @@ -101,8 +107,11 @@ class XdgActivationTokenV1 : public wayland::XdgActivationTokenV1 private: void set_serial(uint32_t serial, struct wl_resource* seat) override; + void set_app_id(std::string const& app_id) override; + void set_surface(struct wl_resource* surface) override; + void commit() override; std::string token; @@ -113,6 +122,7 @@ class XdgActivationTokenV1 : public wayland::XdgActivationTokenV1 std::optional requesting_surface; }; } +} auto mf::create_xdg_activation_v1( struct wl_display* display, @@ -144,11 +154,11 @@ std::string const& mf::XdgActivationV1::create_token() { try_consume_token(generated); })}; - token.alarm->reschedule_in(TIMEOUT_MS); + token.alarm->reschedule_in(timeout_ms); std::lock_guard guard(lock); pending_tokens.emplace_back(std::move(token)); - return pending_tokens[pending_tokens.size() - 1].token; + return pending_tokens.back().token; } bool mf::XdgActivationV1::try_consume_token(std::string const& token) From 41ed154e96bbf5830eab1527fe0eae7018fcd37a Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 17 Oct 2024 15:37:49 -0400 Subject: [PATCH 3/8] feedback: throwing exception when a token is already in use and checking if an event exists --- .../frontend_wayland/xdg_activation_v1.cpp | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/server/frontend_wayland/xdg_activation_v1.cpp b/src/server/frontend_wayland/xdg_activation_v1.cpp index 90fca581bbe..ec818ba9633 100644 --- a/src/server/frontend_wayland/xdg_activation_v1.cpp +++ b/src/server/frontend_wayland/xdg_activation_v1.cpp @@ -17,10 +17,12 @@ #include "xdg_activation_v1.h" #include "wl_surface.h" #include "wl_seat.h" +#include "wl_client.h" #include "mir/main_loop.h" #include "mir/scene/surface.h" #include "mir/shell/shell.h" #include "mir/shell/focus_controller.h" +#include "mir/wayland/protocol_error.h" #include "mir/log.h" #include #include @@ -120,6 +122,7 @@ class XdgActivationTokenV1 : public wayland::XdgActivationTokenV1 std::optional seat; std::optional app_id; std::optional requesting_surface; + bool used = false; }; } } @@ -259,9 +262,18 @@ void mf::XdgActivationTokenV1::set_surface(struct wl_resource* surface) void mf::XdgActivationTokenV1::commit() { - // In the event of a failure, we send 'done' with a new, invalid token - // which cannot be used later. + if (used) + { + BOOST_THROW_EXCEPTION(mw::ProtocolError( + resource, + Error::already_used, + "The activation token has already been used")); + return; + } + used = true; + + // In the event of a failure, we send 'done' with a new, invalid token which cannot be used later. if (seat && serial) { // First, assert that the seat still exists. @@ -273,8 +285,13 @@ void mf::XdgActivationTokenV1::commit() return; } - // TODO: - // Next, verify that the serial belongs to the seat + auto event = client->event_for(serial.value()); + if (!event) + { + mir::log_error("XdgActivationTokenV1::commit serial not found"); + send_done_event(generate_token()); + return; + } } if (app_id) @@ -309,10 +326,9 @@ void mf::XdgActivationTokenV1::commit() } auto const& scene_surface = scene_surface_opt.value(); - if (focus_controller->focused_surface() != scene_surface) { - mir::log_error("XdgActivationTokenV1::commit the focused surface is not the surface that requesteda ctivation"); + mir::log_error("XdgActivationTokenV1::commit the focused surface is not the surface that requested activation"); send_done_event(generate_token()); return; } From 302787d7be83000dd641dc18057892c0c7b8659e Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 17 Oct 2024 16:09:22 -0400 Subject: [PATCH 4/8] Fixing a bug where the serial could be 0 for some clients and cleaning up the lock guards --- .../frontend_wayland/xdg_activation_v1.cpp | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/server/frontend_wayland/xdg_activation_v1.cpp b/src/server/frontend_wayland/xdg_activation_v1.cpp index ec818ba9633..553caade847 100644 --- a/src/server/frontend_wayland/xdg_activation_v1.cpp +++ b/src/server/frontend_wayland/xdg_activation_v1.cpp @@ -96,7 +96,7 @@ class XdgActivationV1 : public wayland::XdgActivationV1::Global std::shared_ptr const focus_controller; std::shared_ptr main_loop; std::vector pending_tokens; - std::mutex lock; + std::mutex mutex; }; class XdgActivationTokenV1 : public wayland::XdgActivationTokenV1 @@ -159,20 +159,24 @@ std::string const& mf::XdgActivationV1::create_token() })}; token.alarm->reschedule_in(timeout_ms); - std::lock_guard guard(lock); - pending_tokens.emplace_back(std::move(token)); - return pending_tokens.back().token; + { + std::lock_guard guard(mutex); + pending_tokens.emplace_back(std::move(token)); + return pending_tokens.back().token; + } } bool mf::XdgActivationV1::try_consume_token(std::string const& token) { - std::lock_guard guard(lock); - for (auto it = pending_tokens.begin(); it != pending_tokens.end(); it++) { - if (it->token == token) + std::lock_guard guard(mutex); + for (auto it = pending_tokens.begin(); it != pending_tokens.end(); it++) { - pending_tokens.erase(it); - return true; + if (it->token == token) + { + pending_tokens.erase(it); + return true; + } } } @@ -246,7 +250,8 @@ mf::XdgActivationTokenV1::XdgActivationTokenV1( void mf::XdgActivationTokenV1::set_serial(uint32_t serial_, struct wl_resource* seat_) { - serial = serial_; + if (serial_ != 0) + serial = serial_; seat = seat_; } From e6be816ee31fe69833a9bc4ccb1ec4d5e0b0e3a6 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Fri, 18 Oct 2024 14:48:37 -0400 Subject: [PATCH 5/8] Validating input in the activation method rather than the commit, and making sure that tokens are invalidated if a keyboard event comes in --- .../frontend_wayland/wayland_connector.cpp | 3 +- .../frontend_wayland/wayland_connector.h | 1 + .../wayland_default_configuration.cpp | 8 +- .../frontend_wayland/xdg_activation_v1.cpp | 260 +++++++++++------- .../frontend_wayland/xdg_activation_v1.h | 9 +- 5 files changed, 172 insertions(+), 109 deletions(-) diff --git a/src/server/frontend_wayland/wayland_connector.cpp b/src/server/frontend_wayland/wayland_connector.cpp index 632809bd625..f2aefe61b36 100644 --- a/src/server/frontend_wayland/wayland_connector.cpp +++ b/src/server/frontend_wayland/wayland_connector.cpp @@ -330,7 +330,8 @@ mf::WaylandConnector::WaylandConnector( desktop_file_manager, session_lock_, decoration_strategy, - focus_controller}); + focus_controller, + keyboard_observer_registrar}); shm_global = std::make_unique(display.get(), executor); diff --git a/src/server/frontend_wayland/wayland_connector.h b/src/server/frontend_wayland/wayland_connector.h index 9ab92a53385..22ee33d8868 100644 --- a/src/server/frontend_wayland/wayland_connector.h +++ b/src/server/frontend_wayland/wayland_connector.h @@ -115,6 +115,7 @@ class WaylandExtensions std::shared_ptr session_lock; std::shared_ptr decoration_strategy; std::shared_ptr focus_controller; + std::shared_ptr> keyboard_observer_registrar; }; WaylandExtensions() = default; diff --git a/src/server/frontend_wayland/wayland_default_configuration.cpp b/src/server/frontend_wayland/wayland_default_configuration.cpp index c4bd0ff0c63..bbd3d2134bc 100644 --- a/src/server/frontend_wayland/wayland_default_configuration.cpp +++ b/src/server/frontend_wayland/wayland_default_configuration.cpp @@ -229,7 +229,13 @@ std::vector const internal_extension_builders = { }), make_extension_builder([](auto const& ctx) { - return mf::create_xdg_activation_v1(ctx.display, ctx.shell, ctx.focus_controller, ctx.main_loop); + return mf::create_xdg_activation_v1( + ctx.display, + ctx.shell, + ctx.focus_controller, + ctx.main_loop, + ctx.keyboard_observer_registrar, + *ctx.wayland_executor); }), }; diff --git a/src/server/frontend_wayland/xdg_activation_v1.cpp b/src/server/frontend_wayland/xdg_activation_v1.cpp index 553caade847..117d166caae 100644 --- a/src/server/frontend_wayland/xdg_activation_v1.cpp +++ b/src/server/frontend_wayland/xdg_activation_v1.cpp @@ -19,6 +19,7 @@ #include "wl_seat.h" #include "wl_client.h" #include "mir/main_loop.h" +#include "mir/input/keyboard_observer.h" #include "mir/scene/surface.h" #include "mir/shell/shell.h" #include "mir/shell/focus_controller.h" @@ -31,6 +32,7 @@ namespace mf = mir::frontend; namespace mw = mir::wayland; namespace msh = mir::shell; +namespace mi = mir::input; namespace { @@ -49,6 +51,27 @@ namespace mir { namespace frontend { +struct XdgActivationTokenData +{ + XdgActivationTokenData( + std::string const& token, + std::unique_ptr alarm_) + : token{token}, + alarm{std::move(alarm_)} + { + alarm->reschedule_in(timeout_ms); + } + + std::string const token; + std::unique_ptr const alarm; + + std::optional serial; + std::optional seat; + std::optional app_id; + std::optional requesting_surface; +}; + + class XdgActivationV1 : public wayland::XdgActivationV1::Global { public: @@ -56,11 +79,14 @@ class XdgActivationV1 : public wayland::XdgActivationV1::Global struct wl_display* display, std::shared_ptr const& shell, std::shared_ptr const& focus_controller, - std::shared_ptr const& main_loop); - - std::string const& create_token(); + std::shared_ptr const& main_loop, + std::shared_ptr> const& keyboard_observer_registrar, + Executor& wayland_executor); + ~XdgActivationV1(); - bool try_consume_token(std::string const& token); + std::shared_ptr const& create_token(); + std::shared_ptr try_consume_token(std::string const& token); + void invalidate_all(); private: class Instance : public wayland::XdgActivationV1 @@ -84,10 +110,15 @@ class XdgActivationV1 : public wayland::XdgActivationV1::Global std::shared_ptr main_loop; }; - struct Token + class KeyboardObserver: public input::KeyboardObserver { - std::string token; - std::unique_ptr alarm; + public: + KeyboardObserver(XdgActivationV1* xdg_activation_v1); + void keyboard_event(std::shared_ptr const& event) override; + void keyboard_focus_set(std::shared_ptr const& surface) override; + + private: + XdgActivationV1* xdg_activation_v1; }; void bind(wl_resource* resource) override; @@ -95,8 +126,10 @@ class XdgActivationV1 : public wayland::XdgActivationV1::Global std::shared_ptr shell; std::shared_ptr const focus_controller; std::shared_ptr main_loop; - std::vector pending_tokens; - std::mutex mutex; + std::shared_ptr> keyboard_observer_registrar; + std::shared_ptr keyboard_observer; + std::vector> pending_tokens; + std::mutex pending_tokens_mutex; }; class XdgActivationTokenV1 : public wayland::XdgActivationTokenV1 @@ -104,8 +137,7 @@ class XdgActivationTokenV1 : public wayland::XdgActivationTokenV1 public: XdgActivationTokenV1( struct wl_resource* resource, - std::string const& token, - std::shared_ptr const& focus_controller); + std::shared_ptr const& token); private: void set_serial(uint32_t serial, struct wl_resource* seat) override; @@ -116,12 +148,7 @@ class XdgActivationTokenV1 : public wayland::XdgActivationTokenV1 void commit() override; - std::string token; - std::shared_ptr const focus_controller; - std::optional serial; - std::optional seat; - std::optional app_id; - std::optional requesting_surface; + std::shared_ptr token; bool used = false; }; } @@ -131,56 +158,73 @@ auto mf::create_xdg_activation_v1( struct wl_display* display, std::shared_ptr const& shell, std::shared_ptr const& focus_controller, - std::shared_ptr const& main_loop) + std::shared_ptr const& main_loop, + std::shared_ptr> const& keyboard_observer_registrar, + Executor& wayland_executor) -> std::shared_ptr { - return std::make_shared(display, shell, focus_controller, main_loop); + return std::make_shared(display, shell, focus_controller, main_loop, keyboard_observer_registrar, wayland_executor); } mf::XdgActivationV1::XdgActivationV1( struct wl_display* display, std::shared_ptr const& shell, std::shared_ptr const& focus_controller, - std::shared_ptr const& main_loop) + std::shared_ptr const& main_loop, + std::shared_ptr> const& keyboard_observer_registrar, + Executor& wayland_executor) : Global(display, Version<1>()), shell{shell}, focus_controller{focus_controller}, - main_loop{main_loop} + main_loop{main_loop}, + keyboard_observer_registrar{keyboard_observer_registrar}, + keyboard_observer{std::make_shared(this)} { + keyboard_observer_registrar->register_interest(keyboard_observer, wayland_executor); +} +mf::XdgActivationV1::~XdgActivationV1() +{ + keyboard_observer_registrar->unregister_interest(*keyboard_observer); } -std::string const& mf::XdgActivationV1::create_token() +std::shared_ptr const& mf::XdgActivationV1::create_token() { auto generated = generate_token(); - Token token{generated, main_loop->create_alarm([this, generated]() + auto token = std::make_shared(generated, main_loop->create_alarm([this, generated]() { try_consume_token(generated); - })}; - token.alarm->reschedule_in(timeout_ms); + })); { - std::lock_guard guard(mutex); + std::lock_guard guard(pending_tokens_mutex); pending_tokens.emplace_back(std::move(token)); - return pending_tokens.back().token; + return pending_tokens.back(); } } -bool mf::XdgActivationV1::try_consume_token(std::string const& token) +std::shared_ptr mf::XdgActivationV1::try_consume_token(std::string const& token) { { - std::lock_guard guard(mutex); + std::lock_guard guard(pending_tokens_mutex); for (auto it = pending_tokens.begin(); it != pending_tokens.end(); it++) { - if (it->token == token) + if (it->get()->token == token) { + auto result = *it; pending_tokens.erase(it); - return true; + return result; } } } - return false; + return nullptr; +} + +void mf::XdgActivationV1::invalidate_all() +{ + std::lock_guard guard(pending_tokens_mutex); + pending_tokens.clear(); } void mf::XdgActivationV1::bind(struct wl_resource* resource) @@ -202,20 +246,39 @@ mf::XdgActivationV1::Instance::Instance( { } +mf::XdgActivationV1::KeyboardObserver::KeyboardObserver(mir::frontend::XdgActivationV1* xdg_activation_v1) + : xdg_activation_v1{xdg_activation_v1} +{} + +void mf::XdgActivationV1::KeyboardObserver::keyboard_event(std::shared_ptr const&) +{ + xdg_activation_v1->invalidate_all(); +} + +void mf::XdgActivationV1::KeyboardObserver::keyboard_focus_set(std::shared_ptr const&) +{ +} + void mf::XdgActivationV1::Instance::get_activation_token(struct wl_resource* id) { - new XdgActivationTokenV1(id, xdg_activation_v1->create_token(), focus_controller); + new XdgActivationTokenV1(id, xdg_activation_v1->create_token()); } void mf::XdgActivationV1::Instance::activate(std::string const& token, struct wl_resource* surface) { - if (!xdg_activation_v1->try_consume_token(token)) + auto xdg_token = xdg_activation_v1->try_consume_token(token); + if (!xdg_token) { mir::log_error("XdgActivationV1::activate invalid token: %s", token.c_str()); return; } - main_loop->enqueue(this, [surface=surface, shell=shell] + main_loop->enqueue(this, [ + surface=surface, + shell=shell, + xdg_token=xdg_token, + client=client, + focus_controller=focus_controller] { auto const wl_surface = mf::WlSurface::from(surface); if (!wl_surface) @@ -231,6 +294,55 @@ void mf::XdgActivationV1::Instance::activate(std::string const& token, struct wl return; } + // In the event of a failure, we send 'done' with a new, invalid token which cannot be used later. + if (xdg_token->seat && xdg_token->serial) + { + // First, assert that the seat still exists. + auto const wl_seat = mf::WlSeat::from(xdg_token->seat.value()); + if (!wl_seat) + mir::log_warning("XdgActivationTokenV1::activate wl_seat not found"); + + auto event = client->event_for(xdg_token->serial.value()); + if (event == std::nullopt) + mir::log_warning("XdgActivationTokenV1::activate serial not found"); + } + + if (xdg_token->app_id) + { + // Check that the focused application has app_id + auto const& focused = focus_controller->focused_surface(); + if (!focused) + { + mir::log_error("XdgActivationTokenV1::activate cannot find the focused surface"); + return; + } + + if (focused->application_id() != xdg_token->app_id) + { + mir::log_error("XdgActivationTokenV1::activate app_id != focused application id"); + return; + } + } + + if (xdg_token->requesting_surface) + { + // Check that the focused surface is the requesting_surface + auto const requesting_wl_surface = mf::WlSurface::from(xdg_token->requesting_surface.value()); + auto const requesting_scene_surface_opt = requesting_wl_surface->scene_surface(); + if (!requesting_scene_surface_opt) + { + mir::log_error("XdgActivationTokenV1::commit cannot find the provided scene surface"); + return; + } + + auto const& scene_surface = requesting_scene_surface_opt.value(); + if (focus_controller->focused_surface() != scene_surface) + { + mir::log_error("XdgActivationTokenV1::commit the focused surface is not the surface that requested activation"); + return; + } + } + auto const& scene_surface = scene_surface_opt.value(); auto const now = std::chrono::steady_clock::now().time_since_epoch(); auto ns = std::chrono::duration_cast(now).count(); @@ -240,29 +352,26 @@ void mf::XdgActivationV1::Instance::activate(std::string const& token, struct wl mf::XdgActivationTokenV1::XdgActivationTokenV1( struct wl_resource* resource, - std::string const& token, - std::shared_ptr const& focus_controller) + std::shared_ptr const& token) : wayland::XdgActivationTokenV1(resource, Version<1>()), - token{token}, - focus_controller{focus_controller} + token{token} { } void mf::XdgActivationTokenV1::set_serial(uint32_t serial_, struct wl_resource* seat_) { - if (serial_ != 0) - serial = serial_; - seat = seat_; + token->serial = serial_; + token->seat = seat_; } void mf::XdgActivationTokenV1::set_app_id(std::string const& app_id_) { - app_id = app_id_; + token->app_id = app_id_; } void mf::XdgActivationTokenV1::set_surface(struct wl_resource* surface) { - requesting_surface = surface; + token->requesting_surface = surface; } void mf::XdgActivationTokenV1::commit() @@ -278,66 +387,5 @@ void mf::XdgActivationTokenV1::commit() used = true; - // In the event of a failure, we send 'done' with a new, invalid token which cannot be used later. - if (seat && serial) - { - // First, assert that the seat still exists. - auto const wl_seat = mf::WlSeat::from(seat.value()); - if (!wl_seat) - { - mir::log_error("XdgActivationTokenV1::commit wl_seat not found"); - send_done_event(generate_token()); - return; - } - - auto event = client->event_for(serial.value()); - if (!event) - { - mir::log_error("XdgActivationTokenV1::commit serial not found"); - send_done_event(generate_token()); - return; - } - } - - if (app_id) - { - // Check that the focused application has app_id - auto const& focused = focus_controller->focused_surface(); - if (!focused) - { - mir::log_error("XdgActivationTokenV1::commit cannot find the focused surface"); - send_done_event(generate_token()); - return; - } - - if (focused->application_id() != app_id) - { - mir::log_error("XdgActivationTokenV1::commit app_id != focused application id"); - send_done_event(generate_token()); - return; - } - } - - if (requesting_surface) - { - // Check that the focused surface is the requesting_surface - auto const wl_surface = mf::WlSurface::from(requesting_surface.value()); - auto const scene_surface_opt = wl_surface->scene_surface(); - if (!scene_surface_opt) - { - mir::log_error("XdgActivationTokenV1::commit cannot find the provided scene surface"); - send_done_event(generate_token()); - return; - } - - auto const& scene_surface = scene_surface_opt.value(); - if (focus_controller->focused_surface() != scene_surface) - { - mir::log_error("XdgActivationTokenV1::commit the focused surface is not the surface that requested activation"); - send_done_event(generate_token()); - return; - } - } - - send_done_event(token); + send_done_event(token->token); } diff --git a/src/server/frontend_wayland/xdg_activation_v1.h b/src/server/frontend_wayland/xdg_activation_v1.h index b6dd5587199..7ff47ae13c4 100644 --- a/src/server/frontend_wayland/xdg_activation_v1.h +++ b/src/server/frontend_wayland/xdg_activation_v1.h @@ -18,6 +18,7 @@ #define MIR_FRONTEND_XDG_ACTIVATION_UNSTABLE_V1_H #include "xdg-activation-v1_wrapper.h" +#include "mir/observer_registrar.h" struct wl_display; @@ -29,6 +30,10 @@ namespace shell class Shell; class FocusController; } +namespace input +{ +class KeyboardObserver; +} namespace frontend { @@ -36,7 +41,9 @@ auto create_xdg_activation_v1( struct wl_display* display, std::shared_ptr const&, std::shared_ptr const&, - std::shared_ptr const&) -> + std::shared_ptr const&, + std::shared_ptr> const&, + Executor& wayland_executor) -> std::shared_ptr; } } From 38b71871b83064eb07f39b30328ef00155cc790e Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Tue, 29 Oct 2024 10:21:15 -0400 Subject: [PATCH 6/8] pr feedback: listening for focus changes and ensuring that the newly focused surface is the same session that requested the token --- .../frontend_wayland/wayland_connector.cpp | 4 +- .../frontend_wayland/wayland_connector.h | 6 +- .../wayland_default_configuration.cpp | 4 +- .../frontend_wayland/xdg_activation_v1.cpp | 170 +++++++++++------- .../frontend_wayland/xdg_activation_v1.h | 7 +- 5 files changed, 119 insertions(+), 72 deletions(-) diff --git a/src/server/frontend_wayland/wayland_connector.cpp b/src/server/frontend_wayland/wayland_connector.cpp index f2aefe61b36..dd88de115b1 100644 --- a/src/server/frontend_wayland/wayland_connector.cpp +++ b/src/server/frontend_wayland/wayland_connector.cpp @@ -242,7 +242,7 @@ mf::WaylandConnector::WaylandConnector( bool enable_key_repeat, std::shared_ptr const& session_lock, std::shared_ptr const& decoration_strategy, - std::shared_ptr const& focus_controller) + std::shared_ptr const& session_coordinator) : extension_filter{extension_filter}, display{wl_display_create(), &cleanup_display}, pause_signal{eventfd(0, EFD_CLOEXEC | EFD_SEMAPHORE)}, @@ -330,7 +330,7 @@ mf::WaylandConnector::WaylandConnector( desktop_file_manager, session_lock_, decoration_strategy, - focus_controller, + session_coordinator, keyboard_observer_registrar}); shm_global = std::make_unique(display.get(), executor); diff --git a/src/server/frontend_wayland/wayland_connector.h b/src/server/frontend_wayland/wayland_connector.h index 22ee33d8868..970355f00fe 100644 --- a/src/server/frontend_wayland/wayland_connector.h +++ b/src/server/frontend_wayland/wayland_connector.h @@ -59,7 +59,6 @@ class DisplayConfigurationObserver; namespace shell { class Shell; -class FocusController; } namespace scene { @@ -68,6 +67,7 @@ class IdleHub; class Surface; class TextInputHub; class SessionLock; +class SessionCoordinator; } namespace time { @@ -114,7 +114,7 @@ class WaylandExtensions std::shared_ptr desktop_file_manager; std::shared_ptr session_lock; std::shared_ptr decoration_strategy; - std::shared_ptr focus_controller; + std::shared_ptr session_coordinator; std::shared_ptr> keyboard_observer_registrar; }; @@ -169,7 +169,7 @@ class WaylandConnector : public Connector bool enable_key_repeat, std::shared_ptr const& session_lock, std::shared_ptr const& decoration_strategy, - std::shared_ptr const& focus_controller); + std::shared_ptr const& session_coordinator); ~WaylandConnector() override; diff --git a/src/server/frontend_wayland/wayland_default_configuration.cpp b/src/server/frontend_wayland/wayland_default_configuration.cpp index bbd3d2134bc..a829a504e78 100644 --- a/src/server/frontend_wayland/wayland_default_configuration.cpp +++ b/src/server/frontend_wayland/wayland_default_configuration.cpp @@ -232,7 +232,7 @@ std::vector const internal_extension_builders = { return mf::create_xdg_activation_v1( ctx.display, ctx.shell, - ctx.focus_controller, + ctx.session_coordinator, ctx.main_loop, ctx.keyboard_observer_registrar, *ctx.wayland_executor); @@ -396,7 +396,7 @@ std::shared_ptr enable_repeat, the_session_lock(), the_decoration_strategy(), - the_focus_controller()); + the_session_coordinator()); }); } diff --git a/src/server/frontend_wayland/xdg_activation_v1.cpp b/src/server/frontend_wayland/xdg_activation_v1.cpp index 117d166caae..318a0cf06d3 100644 --- a/src/server/frontend_wayland/xdg_activation_v1.cpp +++ b/src/server/frontend_wayland/xdg_activation_v1.cpp @@ -21,16 +21,21 @@ #include "mir/main_loop.h" #include "mir/input/keyboard_observer.h" #include "mir/scene/surface.h" +#include "mir/scene/session_listener.h" +#include "mir/scene/session_coordinator.h" #include "mir/shell/shell.h" -#include "mir/shell/focus_controller.h" #include "mir/wayland/protocol_error.h" #include "mir/log.h" #include #include #include +#include + +#include "../../../examples/client/wayland_runner.h" namespace mf = mir::frontend; namespace mw = mir::wayland; +namespace ms = mir::scene; namespace msh = mir::shell; namespace mi = mir::input; @@ -55,20 +60,21 @@ struct XdgActivationTokenData { XdgActivationTokenData( std::string const& token, - std::unique_ptr alarm_) + std::unique_ptr alarm_, + std::shared_ptr const& session) : token{token}, - alarm{std::move(alarm_)} + alarm{std::move(alarm_)}, + session{session} { alarm->reschedule_in(timeout_ms); } std::string const token; std::unique_ptr const alarm; + std::weak_ptr session; std::optional serial; std::optional seat; - std::optional app_id; - std::optional requesting_surface; }; @@ -78,15 +84,16 @@ class XdgActivationV1 : public wayland::XdgActivationV1::Global XdgActivationV1( struct wl_display* display, std::shared_ptr const& shell, - std::shared_ptr const& focus_controller, + std::shared_ptr const& session_coordinator, std::shared_ptr const& main_loop, std::shared_ptr> const& keyboard_observer_registrar, Executor& wayland_executor); ~XdgActivationV1(); - std::shared_ptr const& create_token(); + std::shared_ptr const& create_token(std::shared_ptr const& session); std::shared_ptr try_consume_token(std::string const& token); void invalidate_all(); + void invalidate_if_not_from_session(std::shared_ptr const&); private: class Instance : public wayland::XdgActivationV1 @@ -96,7 +103,7 @@ class XdgActivationV1 : public wayland::XdgActivationV1::Global struct wl_resource* resource, mf::XdgActivationV1* xdg_activation_v1, std::shared_ptr const& shell, - std::shared_ptr const& focus_controller, + std::shared_ptr const& session_coordinator, std::shared_ptr const& main_loop); private: @@ -106,7 +113,7 @@ class XdgActivationV1 : public wayland::XdgActivationV1::Global mf::XdgActivationV1* xdg_activation_v1; std::shared_ptr shell; - std::shared_ptr const focus_controller; + std::shared_ptr const session_coordinator; std::shared_ptr main_loop; }; @@ -121,10 +128,33 @@ class XdgActivationV1 : public wayland::XdgActivationV1::Global XdgActivationV1* xdg_activation_v1; }; + class SessionListener : public ms::SessionListener + { + public: + SessionListener(XdgActivationV1* xdg_activation_v1); + void starting(std::shared_ptr const&) override {} + void stopping(std::shared_ptr const&) override {} + void focused(std::shared_ptr const& session) override; + void unfocused() override {} + + void surface_created(ms::Session&, std::shared_ptr const&) override {} + void destroying_surface(ms::Session&, std::shared_ptr const&) override {} + + void buffer_stream_created( + ms::Session&, + std::shared_ptr const&) override {} + void buffer_stream_destroyed( + ms::Session&, + std::shared_ptr const&) override {} + + private: + XdgActivationV1* xdg_activation_v1; + }; + void bind(wl_resource* resource) override; std::shared_ptr shell; - std::shared_ptr const focus_controller; + std::shared_ptr const session_coordinator; std::shared_ptr main_loop; std::shared_ptr> keyboard_observer_registrar; std::shared_ptr keyboard_observer; @@ -157,25 +187,25 @@ class XdgActivationTokenV1 : public wayland::XdgActivationTokenV1 auto mf::create_xdg_activation_v1( struct wl_display* display, std::shared_ptr const& shell, - std::shared_ptr const& focus_controller, + std::shared_ptr const& session_coordinator, std::shared_ptr const& main_loop, std::shared_ptr> const& keyboard_observer_registrar, Executor& wayland_executor) -> std::shared_ptr { - return std::make_shared(display, shell, focus_controller, main_loop, keyboard_observer_registrar, wayland_executor); + return std::make_shared(display, shell, session_coordinator, main_loop, keyboard_observer_registrar, wayland_executor); } mf::XdgActivationV1::XdgActivationV1( struct wl_display* display, std::shared_ptr const& shell, - std::shared_ptr const& focus_controller, + std::shared_ptr const& session_coordinator, std::shared_ptr const& main_loop, std::shared_ptr> const& keyboard_observer_registrar, Executor& wayland_executor) : Global(display, Version<1>()), shell{shell}, - focus_controller{focus_controller}, + session_coordinator{session_coordinator}, main_loop{main_loop}, keyboard_observer_registrar{keyboard_observer_registrar}, keyboard_observer{std::make_shared(this)} @@ -188,13 +218,13 @@ mf::XdgActivationV1::~XdgActivationV1() keyboard_observer_registrar->unregister_interest(*keyboard_observer); } -std::shared_ptr const& mf::XdgActivationV1::create_token() +std::shared_ptr const& mf::XdgActivationV1::create_token(std::shared_ptr const& session) { auto generated = generate_token(); auto token = std::make_shared(generated, main_loop->create_alarm([this, generated]() { try_consume_token(generated); - })); + }), session); { std::lock_guard guard(pending_tokens_mutex); @@ -227,21 +257,30 @@ void mf::XdgActivationV1::invalidate_all() pending_tokens.clear(); } +void mf::XdgActivationV1::invalidate_if_not_from_session(std::shared_ptr const& session) +{ + std::lock_guard guard(pending_tokens_mutex); + std::erase_if(pending_tokens, [&](std::shared_ptr const& token) + { + return token->session.expired() || token->session.lock() != session; + }); +} + void mf::XdgActivationV1::bind(struct wl_resource* resource) { - new Instance{resource, this, shell, focus_controller, main_loop}; + new Instance{resource, this, shell, session_coordinator, main_loop}; } mf::XdgActivationV1::Instance::Instance( struct wl_resource* resource, mf::XdgActivationV1* xdg_activation_v1, std::shared_ptr const& shell, - std::shared_ptr const& focus_controller, + std::shared_ptr const& session_coordinator, std::shared_ptr const& main_loop) : XdgActivationV1(resource, Version<1>()), xdg_activation_v1{xdg_activation_v1}, shell{shell}, - focus_controller{focus_controller}, + session_coordinator{session_coordinator}, main_loop{main_loop} { } @@ -250,22 +289,48 @@ mf::XdgActivationV1::KeyboardObserver::KeyboardObserver(mir::frontend::XdgActiva : xdg_activation_v1{xdg_activation_v1} {} -void mf::XdgActivationV1::KeyboardObserver::keyboard_event(std::shared_ptr const&) +void mf::XdgActivationV1::KeyboardObserver::keyboard_event(std::shared_ptr const& event) { - xdg_activation_v1->invalidate_all(); + // If we encounter a key press event, then we invalidate pending activation tokens + if (event->type() != mir_event_type_input) + return; + + auto input_event = event->to_input(); + if (input_event->input_type() != mir_input_event_type_key) + return; + + auto keyboard_event = input_event->to_keyboard(); + if (keyboard_event->action() == mir_keyboard_action_down) + xdg_activation_v1->invalidate_all(); } void mf::XdgActivationV1::KeyboardObserver::keyboard_focus_set(std::shared_ptr const&) { } +void mf::XdgActivationV1::SessionListener::focused(std::shared_ptr const& session) +{ + xdg_activation_v1->invalidate_if_not_from_session(session); +} + void mf::XdgActivationV1::Instance::get_activation_token(struct wl_resource* id) { - new XdgActivationTokenV1(id, xdg_activation_v1->create_token()); + new XdgActivationTokenV1(id, xdg_activation_v1->create_token(client->client_session())); } void mf::XdgActivationV1::Instance::activate(std::string const& token, struct wl_resource* surface) { + // This function handles requests from clients for activation. + // This will fail if the client cannot find their token in the list. + // A client may not be able to find their token in the list if any + // of the following are true: + // + // 1. A surface other than the surface that originally requested the activation token + // received focus in the meantime, unless it was a layer shell surface that + // initially requested the focus. + // 2. The surface failed to use the token in the alotted period of time + // 3. A key was pressed down between the issuing of the token and the activation + // of the surface with that token auto xdg_token = xdg_activation_v1->try_consume_token(token); if (!xdg_token) { @@ -278,8 +343,14 @@ void mf::XdgActivationV1::Instance::activate(std::string const& token, struct wl shell=shell, xdg_token=xdg_token, client=client, - focus_controller=focus_controller] + session_coordinator=session_coordinator] { + if (xdg_token->session.expired()) + { + mir::log_error("XdgActivationV1::activate requesting session has expired"); + return; + } + auto const wl_surface = mf::WlSurface::from(surface); if (!wl_surface) { @@ -307,42 +378,6 @@ void mf::XdgActivationV1::Instance::activate(std::string const& token, struct wl mir::log_warning("XdgActivationTokenV1::activate serial not found"); } - if (xdg_token->app_id) - { - // Check that the focused application has app_id - auto const& focused = focus_controller->focused_surface(); - if (!focused) - { - mir::log_error("XdgActivationTokenV1::activate cannot find the focused surface"); - return; - } - - if (focused->application_id() != xdg_token->app_id) - { - mir::log_error("XdgActivationTokenV1::activate app_id != focused application id"); - return; - } - } - - if (xdg_token->requesting_surface) - { - // Check that the focused surface is the requesting_surface - auto const requesting_wl_surface = mf::WlSurface::from(xdg_token->requesting_surface.value()); - auto const requesting_scene_surface_opt = requesting_wl_surface->scene_surface(); - if (!requesting_scene_surface_opt) - { - mir::log_error("XdgActivationTokenV1::commit cannot find the provided scene surface"); - return; - } - - auto const& scene_surface = requesting_scene_surface_opt.value(); - if (focus_controller->focused_surface() != scene_surface) - { - mir::log_error("XdgActivationTokenV1::commit the focused surface is not the surface that requested activation"); - return; - } - } - auto const& scene_surface = scene_surface_opt.value(); auto const now = std::chrono::steady_clock::now().time_since_epoch(); auto ns = std::chrono::duration_cast(now).count(); @@ -364,14 +399,24 @@ void mf::XdgActivationTokenV1::set_serial(uint32_t serial_, struct wl_resource* token->seat = seat_; } -void mf::XdgActivationTokenV1::set_app_id(std::string const& app_id_) +void mf::XdgActivationTokenV1::set_app_id(std::string const& app_id) { - token->app_id = app_id_; + // TODO: This is the application id of the surface that is coming up. + // Until it presents itself as a problem, we will ignore it for now. + // Most likely this is supposed to be used that the application who + // is using the application token is the application that we intend + // to use that token. + (void)app_id; } void mf::XdgActivationTokenV1::set_surface(struct wl_resource* surface) { - token->requesting_surface = surface; + // TODO: This is the application id of the requesting surface. + // Until it presents itself as a problem, we will ignore it or now. + // Instead, we only ensure that the same same session is focused + // between token request and activation. Or we simply ensure + // that focus hasn't changed at all. + (void)surface; } void mf::XdgActivationTokenV1::commit() @@ -386,6 +431,5 @@ void mf::XdgActivationTokenV1::commit() } used = true; - send_done_event(token->token); } diff --git a/src/server/frontend_wayland/xdg_activation_v1.h b/src/server/frontend_wayland/xdg_activation_v1.h index 7ff47ae13c4..388f9d1d193 100644 --- a/src/server/frontend_wayland/xdg_activation_v1.h +++ b/src/server/frontend_wayland/xdg_activation_v1.h @@ -28,7 +28,10 @@ class MainLoop; namespace shell { class Shell; -class FocusController; +} +namespace scene +{ +class SessionCoordinator; } namespace input { @@ -40,7 +43,7 @@ namespace frontend auto create_xdg_activation_v1( struct wl_display* display, std::shared_ptr const&, - std::shared_ptr const&, + std::shared_ptr const&, std::shared_ptr const&, std::shared_ptr> const&, Executor& wayland_executor) -> From 1afa05c3a1d98a613ed9547afd85ccd708a3322e Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Tue, 29 Oct 2024 11:46:09 -0400 Subject: [PATCH 7/8] nits: minor pr fedback --- src/server/frontend_wayland/xdg_activation_v1.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/server/frontend_wayland/xdg_activation_v1.cpp b/src/server/frontend_wayland/xdg_activation_v1.cpp index 318a0cf06d3..dd77469d351 100644 --- a/src/server/frontend_wayland/xdg_activation_v1.cpp +++ b/src/server/frontend_wayland/xdg_activation_v1.cpp @@ -31,8 +31,6 @@ #include #include -#include "../../../examples/client/wayland_runner.h" - namespace mf = mir::frontend; namespace mw = mir::wayland; namespace ms = mir::scene; @@ -411,11 +409,10 @@ void mf::XdgActivationTokenV1::set_app_id(std::string const& app_id) void mf::XdgActivationTokenV1::set_surface(struct wl_resource* surface) { - // TODO: This is the application id of the requesting surface. - // Until it presents itself as a problem, we will ignore it or now. - // Instead, we only ensure that the same same session is focused - // between token request and activation. Or we simply ensure - // that focus hasn't changed at all. + // TODO: This is the requesting surface. Until it presents itself + // as a problem, we will ignore it or now. Instead, we only ensure + // that the same same session is focused between token request + // and activation. Or we simply ensure that focus hasn't changed at all. (void)surface; } From ded0522a5fe0a9a8d5b2cacfe195a20c3ab443b2 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Wed, 30 Oct 2024 05:10:47 -0400 Subject: [PATCH 8/8] Enable XdgActivationV1 by default --- src/server/frontend_wayland/wayland_default_configuration.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server/frontend_wayland/wayland_default_configuration.cpp b/src/server/frontend_wayland/wayland_default_configuration.cpp index a829a504e78..fd76d7b1a18 100644 --- a/src/server/frontend_wayland/wayland_default_configuration.cpp +++ b/src/server/frontend_wayland/wayland_default_configuration.cpp @@ -338,6 +338,7 @@ auto mf::get_standard_extensions() -> std::vector mw::TextInputManagerV3::interface_name, mw::MirShellV1::interface_name, mw::XdgDecorationManagerV1::interface_name, + mw::XdgActivationV1::interface_name, mw::FractionalScaleManagerV1::interface_name}; }