Revert previous fix for #299

Commits reverted:
b652e8b58d
bdf809d039
e9ab970989
481ac54f67

While those commits themselves could be useful even if they don't
completely fix #299, they come with the risks of introduce more
unforeseen bugs, which outweigh their benefit, so revert them.

A brief explanation of their problem:

The fix hinges on the destroy event of the client window to work.
However, the client window could be destroyed so fast, before we even
handle the map, or even the create, event of the frame. And we won't be
listening for substructure events on the frame at the point the client
window is destroyed. Thus completely miss the client window destroy
event.

Because of the inherent racy nature of Xorg, this approach is really
difficult to make work.

Fixes #371

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
This commit is contained in:
Yuxuan Shui 2020-04-06 20:22:54 +01:00
parent bdf809d039
commit 04fe4a76b2
No known key found for this signature in database
GPG Key ID: 37C999F617EA1A47
4 changed files with 15 additions and 34 deletions

View File

@ -263,21 +263,8 @@ static inline void ev_configure_notify(session_t *ps, xcb_configure_notify_event
static inline void ev_destroy_notify(session_t *ps, xcb_destroy_notify_event_t *ev) { static inline void ev_destroy_notify(session_t *ps, xcb_destroy_notify_event_t *ev) {
auto w = find_win(ps, ev->window); auto w = find_win(ps, ev->window);
auto mw = find_toplevel(ps, ev->window); if (w) {
if (mw && mw->client_win == mw->base.id) {
// We only want _real_ frame window
assert(&mw->base == w);
mw = NULL;
}
assert(w == NULL || mw == NULL);
if (w != NULL) {
auto _ attr_unused = destroy_win_start(ps, w); auto _ attr_unused = destroy_win_start(ps, w);
} else if (mw != NULL) {
win_recheck_client(ps, mw);
} else {
log_debug("Received a destroy notify from an unknown window, %#010x",
ev->window);
} }
} }
@ -349,13 +336,15 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t
ps->c, ev->window, XCB_CW_EVENT_MASK, ps->c, ev->window, XCB_CW_EVENT_MASK,
(const uint32_t[]){determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN)}); (const uint32_t[]){determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN)});
// Mark the window as the client window of its parent. // Check if the window is an undetected client window
// Firstly, check if it's a known client window
if (!w_top) {
// If not, look for its frame window
auto w_real_top = find_managed_window_or_parent(ps, ev->parent); auto w_real_top = find_managed_window_or_parent(ps, ev->parent);
// If found, and the client window has not been determined, or its // If found, and the client window has not been determined, or its
// frame may not have a correct client, continue // frame may not have a correct client, continue
if (w_real_top) { if (w_real_top && (!w_real_top->client_win ||
if (!w_real_top->client_win || w_real_top->client_win == w_real_top->base.id)) {
w_real_top->client_win == w_real_top->base.id) {
// If it has WM_STATE, mark it the client window // If it has WM_STATE, mark it the client window
if (wid_has_prop(ps, ev->window, ps->atoms->aWM_STATE)) { if (wid_has_prop(ps, ev->window, ps->atoms->aWM_STATE)) {
w_real_top->wmwin = false; w_real_top->wmwin = false;
@ -370,11 +359,6 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t
determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN) | determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN) |
XCB_EVENT_MASK_PROPERTY_CHANGE}); XCB_EVENT_MASK_PROPERTY_CHANGE});
} }
} else {
log_warn("Window %#010x reparented to a window that "
"already has a client window, the parent is "
"%#010x (%s)",
ev->window, w_real_top->base.id, w_real_top->name);
} }
} }
} }

View File

@ -303,9 +303,9 @@ uint32_t determine_evmask(session_t *ps, xcb_window_t wid, win_evmode_t mode) {
struct managed_win *w = NULL; struct managed_win *w = NULL;
// Check if it's a mapped frame window // Check if it's a mapped frame window
if (mode == WIN_EVMODE_FRAME || if (WIN_EVMODE_FRAME == mode ||
((w = find_managed_win(ps, wid)) && w->a.map_state == XCB_MAP_STATE_VIEWABLE)) { ((w = find_managed_win(ps, wid)) && w->a.map_state == XCB_MAP_STATE_VIEWABLE)) {
evmask |= XCB_EVENT_MASK_PROPERTY_CHANGE | XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY; evmask |= XCB_EVENT_MASK_PROPERTY_CHANGE;
if (!ps->o.use_ewmh_active_win) { if (!ps->o.use_ewmh_active_win) {
evmask |= XCB_EVENT_MASK_FOCUS_CHANGE; evmask |= XCB_EVENT_MASK_FOCUS_CHANGE;
} }

View File

@ -1037,8 +1037,6 @@ void win_mark_client(session_t *ps, struct managed_win *w, xcb_window_t client)
*/ */
void win_unmark_client(session_t *ps, struct managed_win *w) { void win_unmark_client(session_t *ps, struct managed_win *w) {
xcb_window_t client = w->client_win; xcb_window_t client = w->client_win;
log_debug("Detaching client window %#010x from frame %#010x (%s)", client,
w->base.id, w->name);
w->client_win = XCB_NONE; w->client_win = XCB_NONE;
@ -1054,7 +1052,7 @@ void win_unmark_client(session_t *ps, struct managed_win *w) {
* @param ps current session * @param ps current session
* @param w struct _win of the parent window * @param w struct _win of the parent window
*/ */
void win_recheck_client(session_t *ps, struct managed_win *w) { static void win_recheck_client(session_t *ps, struct managed_win *w) {
// Initialize wmwin to false // Initialize wmwin to false
w->wmwin = false; w->wmwin = false;
@ -1064,14 +1062,14 @@ void win_recheck_client(session_t *ps, struct managed_win *w) {
// sets override-redirect flags on all frame windows. // sets override-redirect flags on all frame windows.
xcb_window_t cw = find_client_win(ps, w->base.id); xcb_window_t cw = find_client_win(ps, w->base.id);
if (cw) { if (cw) {
log_debug("(%#010x): client %#010x", w->base.id, cw); log_trace("(%#010x): client %#010x", w->base.id, cw);
} }
// Set a window's client window to itself if we couldn't find a // Set a window's client window to itself if we couldn't find a
// client window // client window
if (!cw) { if (!cw) {
cw = w->base.id; cw = w->base.id;
w->wmwin = !w->a.override_redirect; w->wmwin = !w->a.override_redirect;
log_debug("(%#010x): client self (%s)", w->base.id, log_trace("(%#010x): client self (%s)", w->base.id,
(w->wmwin ? "wmwin" : "override-redirected")); (w->wmwin ? "wmwin" : "override-redirected"));
} }

View File

@ -299,7 +299,6 @@ void win_on_win_size_change(session_t *ps, struct managed_win *w);
void win_update_wintype(session_t *ps, struct managed_win *w); void win_update_wintype(session_t *ps, struct managed_win *w);
void win_mark_client(session_t *ps, struct managed_win *w, xcb_window_t client); void win_mark_client(session_t *ps, struct managed_win *w, xcb_window_t client);
void win_unmark_client(session_t *ps, struct managed_win *w); void win_unmark_client(session_t *ps, struct managed_win *w);
void win_recheck_client(session_t *ps, struct managed_win *w);
bool win_get_class(session_t *ps, struct managed_win *w); bool win_get_class(session_t *ps, struct managed_win *w);
/** /**