wm: make hover handling robust against input races

The window manager infers the overall state from the intercepted input
events for the decorator and all GUI clients. However, each of those
parties have an independent input-event stream. Whereas the order of
events within one GUI session is strict, the order of events between GUI
sessions is arbitrary. The window manager wrongly relied on a global
event ordering to track the pointed-at GUI session.

The patch removes the assumption of a global event order by tracking the
relevant pointer state for each GUI session independently and evaluating
these states when propagating the pointer position to the decorator.

Fixes #4059
This commit is contained in:
Norman Feske 2021-03-26 14:47:58 +01:00
parent bfea27a258
commit f839b3ecba
5 changed files with 166 additions and 144 deletions

View File

@ -17,7 +17,6 @@
/* Genode includes */
#include <util/string.h>
#include <base/attached_dataspace.h>
#include <os/reporter.h>
#include <gui_session/connection.h>
#include <input_session/client.h>
#include <input/event.h>
@ -25,7 +24,7 @@
/* local includes */
#include <window_registry.h>
#include <last_motion.h>
#include <pointer.h>
namespace Wm { class Main;
using Genode::size_t;
@ -173,18 +172,12 @@ struct Wm::Decorator_gui_session : Genode::Rpc_object<Gui::Session>,
Command_buffer &_command_buffer = *_command_ds.local_addr<Command_buffer>();
Point _pointer_position { };
Reporter &_pointer_reporter;
Last_motion &_last_motion;
Pointer::State _pointer_state;
Input::Session_component &_window_layouter_input;
Decorator_content_callback &_content_callback;
unsigned _key_cnt = 0;
/* XXX don't allocate content-registry entries from heap */
Decorator_content_registry _content_registry { _heap };
@ -206,15 +199,13 @@ struct Wm::Decorator_gui_session : Genode::Rpc_object<Gui::Session>,
Decorator_gui_session(Genode::Env &env,
Genode::Ram_allocator &ram,
Allocator &md_alloc,
Reporter &pointer_reporter,
Last_motion &last_motion,
Pointer::Tracker &pointer_tracker,
Input::Session_component &window_layouter_input,
Decorator_content_callback &content_callback)
:
_env(env),
_ram(ram),
_pointer_reporter(pointer_reporter),
_last_motion(last_motion),
_pointer_state(pointer_tracker),
_window_layouter_input(window_layouter_input),
_content_callback(content_callback),
_md_alloc(md_alloc)
@ -229,62 +220,10 @@ struct Wm::Decorator_gui_session : Genode::Rpc_object<Gui::Session>,
void _handle_input()
{
auto report_pointer_position = [&] () {
Reporter::Xml_generator xml(_pointer_reporter, [&] ()
{
xml.attribute("xpos", _pointer_position.x());
xml.attribute("ypos", _pointer_position.y());
});
};
while (_gui_session.input()->pending())
_gui_session.input()->for_each_event([&] (Input::Event const &ev) {
if (ev.press()) _key_cnt++;
if (ev.release()) {
_key_cnt--;
/*
* When returning from a drag operation to idle state, the
* pointer position may have moved to another window
* element. Propagate the least recent pointer position to
* the decorator to update its hover model.
*/
if (_key_cnt == 0)
report_pointer_position();
}
ev.handle_absolute_motion([&] (int x, int y) {
_last_motion = LAST_MOTION_DECORATOR;
_pointer_position = Point(x, y);
/* update pointer model, but only when hovering */
if (_key_cnt == 0)
report_pointer_position();
});
if (ev.hover_leave()) {
/*
* Invalidate pointer as reported to the decorator if the
* pointer moved from a window decoration to a position
* with no window known to the window manager. If the last
* motion referred to one of the regular client session,
* this is not needed because the respective session will
* update the pointer model with the entered position
* already.
*/
if (_last_motion == LAST_MOTION_DECORATOR) {
Reporter::Xml_generator xml(_pointer_reporter, [&] ()
{ });
}
}
_window_layouter_input.submit(ev);
});
_pointer_state.apply_event(ev);
_window_layouter_input.submit(ev); });
}
void _execute_command(Command const &cmd)
@ -393,6 +332,11 @@ struct Wm::Decorator_gui_session : Genode::Rpc_object<Gui::Session>,
_gui_session.upgrade_ram(ram_quota);
}
Pointer::Position last_observed_pointer_pos() const
{
return _pointer_state.last_observed_pos();
}
/***************************
** GUI session interface **

View File

@ -85,7 +85,6 @@ namespace Wm { namespace Gui {
struct Wm::Gui::Click_handler : Interface
{
virtual void handle_click(Point pos) = 0;
virtual void handle_enter(Point pos) = 0;
};
@ -491,6 +490,7 @@ class Wm::Gui::Session_component : public Rpc_object<Gui::Session>,
Area _requested_size { };
bool _resize_requested = false;
bool _has_alpha = false;
Pointer::State _pointer_state;
Point const _initial_pointer_pos { -1, -1 };
Point _pointer_pos = _initial_pointer_pos;
Point _virtual_pointer_pos { };
@ -594,30 +594,33 @@ class Wm::Gui::Session_component : public Rpc_object<Gui::Session>,
_click_handler.handle_click(_pointer_pos);
/*
* Reset pointer model for the decorator once the pointer
* enters the application area of a window.
* Hide application-local motion events from the pointer
* position shared with the decorator. The position is
* propagated only when entering/leaving an application's
* screen area or when finishing a drag operation.
*/
bool propagate_to_pointer_state = false;
/* pointer enters application area */
if (ev.absolute_motion() && _first_motion && _key_cnt == 0) {
_click_handler.handle_enter(_pointer_pos);
propagate_to_pointer_state = true;
_first_motion = false;
}
/*
* We may leave the dragged state on another window than
* the clicked one. During the dragging, the decorator
* remained unaware of pointer movements. Now, when leaving
* the drag stage, we need to make the decorator aware of
* the most recent pointer position to update the hover
* model.
*/
/* may be end of drag operation */
if (ev.release() && _key_cnt == 0)
_click_handler.handle_enter(_pointer_pos);
propagate_to_pointer_state = true;
/* pointer has left the application area */
if (ev.hover_leave()) {
_pointer_pos = _initial_pointer_pos;
_first_motion = true;
propagate_to_pointer_state = true;
}
if (propagate_to_pointer_state)
_pointer_state.apply_event(ev);
/* submit event to the client */
_input_session.submit(_translate_event(ev, input_origin));
}
@ -775,6 +778,7 @@ class Wm::Gui::Session_component : public Rpc_object<Gui::Session>,
Window_registry &window_registry,
Allocator &session_alloc,
Session_label const &session_label,
Pointer::Tracker &pointer_tracker,
Click_handler &click_handler,
Session_control_fn &session_control_fn)
:
@ -787,6 +791,7 @@ class Wm::Gui::Session_component : public Rpc_object<Gui::Session>,
_child_view_alloc(&session_alloc),
_input_session_cap(env.ep().manage(_input_session)),
_click_handler(click_handler),
_pointer_state(pointer_tracker),
_view_handle_registry(session_alloc)
{
_gui_input.sigh(_input_handler);
@ -886,6 +891,11 @@ class Wm::Gui::Session_component : public Rpc_object<Gui::Session>,
v->hidden(hidden);
}
Pointer::Position last_observed_pointer_pos() const
{
return _pointer_state.last_observed_pos();
}
/**
* Return session capability to real GUI session
*/
@ -1070,14 +1080,12 @@ class Wm::Gui::Root : public Genode::Rpc_object<Genode::Typed_root<Session> >,
enum { STACK_SIZE = 1024*sizeof(long) };
Reporter &_pointer_reporter;
Pointer::Tracker &_pointer_tracker;
Reporter &_focus_request_reporter;
unsigned _focus_request_cnt = 0;
Last_motion _last_motion = LAST_MOTION_DECORATOR;
Window_registry &_window_registry;
Input::Session_component _window_layouter_input { _env, _env.ram() };
@ -1088,32 +1096,9 @@ class Wm::Gui::Root : public Genode::Rpc_object<Genode::Typed_root<Session> >,
struct Click_handler : Gui::Click_handler
{
Input::Session_component &window_layouter_input;
Reporter &pointer_reporter;
Last_motion &last_motion;
void handle_enter(Gui::Point pos) override
{
last_motion = LAST_MOTION_NITPICKER;
Reporter::Xml_generator xml(pointer_reporter, [&] ()
{
xml.attribute("xpos", pos.x());
xml.attribute("ypos", pos.y());
});
}
void handle_click(Gui::Point pos) override
{
/*
* Propagate clicked-at position to decorator such that it can
* update its hover model.
*/
Reporter::Xml_generator xml(pointer_reporter, [&] ()
{
xml.attribute("xpos", pos.x());
xml.attribute("ypos", pos.y());
});
/*
* Supply artificial mouse click to the decorator's input session
* (which is routed to the layouter).
@ -1123,17 +1108,10 @@ class Wm::Gui::Root : public Genode::Rpc_object<Genode::Typed_root<Session> >,
window_layouter_input.submit(Input::Release{Input::BTN_LEFT});
}
Click_handler(Input::Session_component &window_layouter_input,
Reporter &pointer_reporter,
Last_motion &last_motion)
:
window_layouter_input(window_layouter_input),
pointer_reporter(pointer_reporter),
last_motion(last_motion)
{ }
Click_handler(Input::Session_component &window_layouter_input)
: window_layouter_input(window_layouter_input) { }
} _click_handler { _window_layouter_input, _pointer_reporter,
_last_motion };
} _click_handler { _window_layouter_input };
/**
* List of regular sessions
@ -1157,12 +1135,12 @@ class Wm::Gui::Root : public Genode::Rpc_object<Genode::Typed_root<Session> >,
Root(Genode::Env &env,
Window_registry &window_registry, Allocator &md_alloc,
Genode::Ram_allocator &ram,
Reporter &pointer_reporter, Reporter &focus_request_reporter,
Pointer::Tracker &pointer_tracker, Reporter &focus_request_reporter,
Gui::Session &focus_gui_session)
:
_env(env),
_md_alloc(md_alloc), _ram(ram),
_pointer_reporter(pointer_reporter),
_pointer_tracker(pointer_tracker),
_focus_request_reporter(focus_request_reporter),
_window_registry(window_registry),
_focus_gui_session(focus_gui_session)
@ -1172,6 +1150,24 @@ class Wm::Gui::Root : public Genode::Rpc_object<Genode::Typed_root<Session> >,
env.parent().announce(env.ep().manage(*this));
}
Pointer::Position last_observed_pointer_pos() const
{
Pointer::Position pos { };
for (Decorator_gui_session const *s = _decorator_sessions.first(); s; s = s->next()) {
if (!pos.valid)
pos = s->last_observed_pointer_pos(); };
if (pos.valid)
return pos;
for (Session_component const *s = _sessions.first(); s; s = s->next()) {
if (!pos.valid)
pos = s->last_observed_pointer_pos(); };
return pos;
}
/********************
** Root interface **
@ -1215,6 +1211,7 @@ class Wm::Gui::Root : public Genode::Rpc_object<Genode::Typed_root<Session> >,
auto session = new (_md_alloc)
Session_component(_env, _ram, _window_registry,
_md_alloc, session_label,
_pointer_tracker,
_click_handler, *this);
_sessions.insert(session);
return _env.ep().manage(*session);
@ -1224,8 +1221,7 @@ class Wm::Gui::Root : public Genode::Rpc_object<Genode::Typed_root<Session> >,
{
auto session = new (_md_alloc)
Decorator_gui_session(_env, _ram, _md_alloc,
_pointer_reporter,
_last_motion,
_pointer_tracker,
_window_layouter_input,
*this);
_decorator_sessions.insert(session);

View File

@ -1,22 +0,0 @@
/*
* \brief Type of client that receive the last motion event
* \author Norman Feske
* \date 2014-06-10
*/
/*
* Copyright (C) 2014-2017 Genode Labs GmbH
*
* This file is part of the Genode OS framework, which is distributed
* under the terms of the GNU Affero General Public License version 3.
*/
#ifndef _LAST_MOTION_H_
#define _LAST_MOTION_H_
namespace Wm {
enum Last_motion { LAST_MOTION_NITPICKER, LAST_MOTION_DECORATOR };
}
#endif /* _LAST_MOTION_H_ */

View File

@ -37,7 +37,7 @@ namespace Wm {
}
struct Wm::Main
struct Wm::Main : Pointer::Tracker
{
Genode::Env &env;
@ -63,7 +63,7 @@ struct Wm::Main
Gui::Connection focus_gui_session { env };
Gui::Root gui_root { env, window_registry, heap, env.ram(),
pointer_reporter, focus_request_reporter,
*this, focus_request_reporter,
focus_gui_session };
void handle_focus_update()
@ -112,6 +112,22 @@ struct Wm::Main
Report_forwarder _report_forwarder { env, heap };
Rom_forwarder _rom_forwarder { env, heap };
/**
* Pointer::Tracker interface
*/
void update_pointer_report() override
{
Pointer::Position pos = gui_root.last_observed_pointer_pos();
Reporter::Xml_generator xml(pointer_reporter, [&] ()
{
if (pos.valid) {
xml.attribute("xpos", pos.value.x());
xml.attribute("ypos", pos.value.y());
}
});
}
Main(Genode::Env &env) : env(env)
{
pointer_reporter.enabled(true);

View File

@ -0,0 +1,88 @@
/*
* \brief Type of client that receive the last motion event
* \author Norman Feske
* \date 2014-06-10
*/
/*
* Copyright (C) 2014-2017 Genode Labs GmbH
*
* This file is part of the Genode OS framework, which is distributed
* under the terms of the GNU Affero General Public License version 3.
*/
#ifndef _POINTER_H_
#define _POINTER_H_
#include <util/noncopyable.h>
namespace Wm { struct Pointer; }
struct Wm::Pointer
{
struct Position
{
bool valid;
Point value;
};
struct Tracker : Genode::Interface, Genode::Noncopyable
{
virtual void update_pointer_report() = 0;
};
class State : Genode::Noncopyable
{
private:
Position _last_observed { };
unsigned _key_cnt = 0;
Tracker &_tracker;
public:
State(Tracker &tracker) : _tracker(tracker) { }
void apply_event(Input::Event const &ev)
{
bool pointer_report_update_needed = false;
if (ev.hover_leave())
_last_observed = { .valid = false, .value = { } };
ev.handle_absolute_motion([&] (int x, int y) {
_last_observed = { .valid = true, .value = { x, y } }; });
if (ev.absolute_motion() || ev.hover_leave())
pointer_report_update_needed = true;
if (ev.press())
_key_cnt++;
if (ev.release()) {
_key_cnt--;
/*
* When returning from a drag operation to idle state, the
* pointer position may have moved to another window
* element. Propagate the least recent pointer position to
* the decorator to update its hover model.
*/
if (_key_cnt == 0)
pointer_report_update_needed = true;
}
if (pointer_report_update_needed)
_tracker.update_pointer_report();
}
Position last_observed_pos() const { return _last_observed; }
};
};
#endif /* _POINTER_H_ */