Merge pull request #377 from yshui/issue299

Fixes #299

src/event.c

@@ -180,8 +180,9 @@ static inline void ev_focus_out(session_t *ps, xcb_focus_out_event_t *ev) {
 }
 
 static inline void ev_create_notify(session_t *ps, xcb_create_notify_event_t *ev) {
-	assert(ev->parent == ps->root);
+	if (ev->parent == ps->root) {
 		add_win_top(ps, ev->window);
+	}
 }
 
 /// Handle configure event of a regular window
@@ -263,8 +264,21 @@ 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) {
 	auto w = find_win(ps, ev->window);
-	if (w) {
+	auto mw = find_toplevel(ps, ev->window);
+	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);
+	} else if (mw != NULL) {
+		win_recheck_client(ps, mw);
+	} else {
+		log_debug("Received a destroy notify from an unknown window, %#010x",
+		          ev->window);
 	}
 }
 
@@ -306,7 +320,8 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t
 	          ev->window, ev->parent, ev->override_redirect);
 	auto w_top = find_toplevel(ps, ev->window);
 	if (w_top) {
-		win_unmark_client(ps, w_top);
+		win_set_flags(w_top, WIN_FLAGS_CLIENT_STALE);
+		ps->pending_updates = true;
 	}
 
 	if (ev->parent == ps->root) {
@@ -336,28 +351,35 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t
 		    ps->c, ev->window, XCB_CW_EVENT_MASK,
 		    (const uint32_t[]){determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN)});
 
-		// Check if the window is an undetected client window
+		if (!wid_has_prop(ps, ev->window, ps->atoms->aWM_STATE)) {
-		// Firstly, check if it's a known client window
+			log_debug("Window %#010x doesn't have WM_STATE property, it is "
-		if (!w_top) {
+			          "probably not a client window. But we will listen for "
-			// If not, look for its frame window
+			          "property change in case it gains one.",
-			auto w_real_top = find_managed_window_or_parent(ps, ev->parent);
+			          ev->window);
-			// If found, and the client window has not been determined, or its
-			// frame may not have a correct client, continue
-			if (w_real_top && (!w_real_top->client_win ||
-			                   w_real_top->client_win == w_real_top->base.id)) {
-				// If it has WM_STATE, mark it the client window
-				if (wid_has_prop(ps, ev->window, ps->atoms->aWM_STATE)) {
-					w_real_top->wmwin = false;
-					win_unmark_client(ps, w_real_top);
-					win_mark_client(ps, w_real_top, ev->window);
-				}
-				// Otherwise, watch for WM_STATE on it
-				else {
 			xcb_change_window_attributes(
 			    ps->c, ev->window, XCB_CW_EVENT_MASK,
-					    (const uint32_t[]){
+			    (const uint32_t[]){determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN) |
-					        determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN) |
 			                       XCB_EVENT_MASK_PROPERTY_CHANGE});
+		} else {
+			auto w_real_top = find_managed_window_or_parent(ps, ev->parent);
+			if (w_real_top && w_real_top->state != WSTATE_UNMAPPED &&
+			    w_real_top->state != WSTATE_UNMAPPING) {
+				log_debug("Mark window %#010x (%s) as having a stale "
+				          "client",
+				          w_real_top->base.id, w_real_top->name);
+				win_set_flags(w_real_top, WIN_FLAGS_CLIENT_STALE);
+				ps->pending_updates = true;
+			} else {
+				if (!w_real_top)
+					log_debug("parent %#010x not found", ev->parent);
+				else {
+					// Window is not currently mapped, unmark its
+					// client to trigger a client recheck when it is
+					// mapped later.
+					win_unmark_client(ps, w_real_top);
+					log_debug("parent %#010x (%s) is in state %d",
+					          w_real_top->base.id, w_real_top->name,
+					          w_real_top->state);
 				}
 			}
 		}
@@ -451,13 +473,11 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t
 			                                 ps, ev->window, WIN_EVMODE_UNKNOWN)});
 
 			auto w_top = find_managed_window_or_parent(ps, ev->window);
-			// Initialize client_win as early as possible
+			// ev->window might have not been managed yet, in that case w_top
-			if (w_top &&
+			// would be NULL.
-			    (!w_top->client_win || w_top->client_win == w_top->base.id) &&
+			if (w_top) {
-			    wid_has_prop(ps, ev->window, ps->atoms->aWM_STATE)) {
+				win_set_flags(w_top, WIN_FLAGS_CLIENT_STALE);
-				w_top->wmwin = false;
+				ps->pending_updates = true;
-				win_unmark_client(ps, w_top);
-				win_mark_client(ps, w_top, ev->window);
 			}
 		}
 	}

src/picom.c

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

src/win.c

@@ -323,8 +323,8 @@ void win_release_images(struct backend_base *backend, struct managed_win *w) {
 void win_process_flags(session_t *ps, struct managed_win *w) {
 	if (win_check_flags_all(w, WIN_FLAGS_MAPPED)) {
 		map_win_start(ps, w);
-	}
 		win_clear_flags(w, WIN_FLAGS_MAPPED);
+	}
 
 	// Not a loop
 	while (win_check_flags_any(w, WIN_FLAGS_IMAGES_STALE) &&
@@ -367,7 +367,14 @@ void win_process_flags(session_t *ps, struct managed_win *w) {
 	}
 
 	// Clear stale image flags
+	if (win_check_flags_any(w, WIN_FLAGS_IMAGES_STALE)) {
 		win_clear_flags(w, WIN_FLAGS_IMAGES_STALE);
+	}
+
+	if (win_check_flags_all(w, WIN_FLAGS_CLIENT_STALE)) {
+		win_recheck_client(ps, w);
+		win_clear_flags(w, WIN_FLAGS_CLIENT_STALE);
+	}
 }
 
 /**
@@ -897,6 +904,7 @@ void win_update_opacity_rule(session_t *ps, struct managed_win *w) {
  * TODO need better name
  */
 void win_on_factor_change(session_t *ps, struct managed_win *w) {
+	log_debug("Window %#010x (%s) factor change", w->base.id, w->name);
 	// Focus needs to be updated first, as other rules might depend on the focused
 	// state of the window
 	win_update_focused(ps, w);
@@ -904,6 +912,8 @@ void win_on_factor_change(session_t *ps, struct managed_win *w) {
 	win_determine_shadow(ps, w);
 	win_determine_invert_color(ps, w);
 	win_determine_blur_background(ps, w);
+	w->mode = win_calc_mode(w);
+	log_debug("Window mode changed to %d", w->mode);
 	win_update_opacity_rule(ps, w);
 	if (w->a.map_state == XCB_MAP_STATE_VIEWABLE)
 		w->paint_excluded = c2_match(ps, w, ps->o.paint_blacklist, NULL);
@@ -1022,6 +1032,8 @@ 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) {
 	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;
 
@@ -1065,7 +1077,7 @@ static xcb_window_t find_client_win(session_t *ps, xcb_window_t w) {
  * @param ps current session
  * @param w struct _win of the parent window
  */
-static void win_recheck_client(session_t *ps, struct managed_win *w) {
+void win_recheck_client(session_t *ps, struct managed_win *w) {
 	// Initialize wmwin to false
 	w->wmwin = false;
 
@@ -1075,14 +1087,14 @@ static void win_recheck_client(session_t *ps, struct managed_win *w) {
 	// sets override-redirect flags on all frame windows.
 	xcb_window_t cw = find_client_win(ps, w->base.id);
 	if (cw) {
-		log_trace("(%#010x): client %#010x", w->base.id, cw);
+		log_debug("(%#010x): client %#010x", w->base.id, cw);
 	}
 	// Set a window's client window to itself if we couldn't find a
 	// client window
 	if (!cw) {
 		cw = w->base.id;
 		w->wmwin = !w->a.override_redirect;
-		log_trace("(%#010x): client self (%s)", w->base.id,
+		log_debug("(%#010x): client self (%s)", w->base.id,
 		          (w->wmwin ? "wmwin" : "override-redirected"));
 	}
 
@@ -2326,6 +2338,7 @@ win_is_fullscreen_xcb(xcb_connection_t *c, const struct atom *a, const xcb_windo
 
 /// Set flags on a window. Some sanity checks are performed
 void win_set_flags(struct managed_win *w, uint64_t flags) {
+	log_debug("Set flags %lu to window %#010x (%s)", flags, w->base.id, w->name);
 	if (unlikely(w->state == WSTATE_DESTROYING)) {
 		log_error("Flags set on a destroyed window %#010x (%s)", w->base.id, w->name);
 		return;
@@ -2336,6 +2349,7 @@ void win_set_flags(struct managed_win *w, uint64_t flags) {
 
 /// Clear flags on a window. Some sanity checks are performed
 void win_clear_flags(struct managed_win *w, uint64_t flags) {
+	log_debug("Clear flags %lu from window %#010x (%s)", flags, w->base.id, w->name);
 	if (unlikely(w->state == WSTATE_DESTROYING)) {
 		log_error("Flags cleared on a destroyed window %#010x (%s)", w->base.id,
 		          w->name);

src/win.h

@@ -298,6 +298,7 @@ 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_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_recheck_client(session_t *ps, struct managed_win *w);
 bool win_get_class(session_t *ps, struct managed_win *w);
 
 /**

src/win_defs.h

@@ -81,6 +81,8 @@ enum win_flags {
 	WIN_FLAGS_SHADOW_STALE = 8,
 	/// shadow has not been generated
 	WIN_FLAGS_SHADOW_NONE = 16,
+	/// the client window needs to be updated
+	WIN_FLAGS_CLIENT_STALE = 32,
 	/// the window is mapped by X, we need to call map_win_start for it
 	WIN_FLAGS_MAPPED = 64,
 };

tests/run_one_test.sh

@@ -7,7 +7,7 @@ fi
 echo "Running test $2"
 
 # TODO keep the log file, and parse it to see if test is successful
-($1 --experimental-backends --backend dummy --log-level=debug --log-file=$PWD/log --config=$2) &
+($1 --dbus --experimental-backends --backend dummy --log-level=debug --log-file=$PWD/log --config=$2) &
 main_pid=$!
 $3
 

tests/run_tests.sh

@@ -3,6 +3,8 @@ set -e
 exe=$(realpath $1)
 cd $(dirname $0)
 
+eval `dbus-launch --sh-syntax`
+
 ./run_one_test.sh $exe configs/empty.conf testcases/basic.py
 ./run_one_test.sh $exe configs/issue357.conf testcases/issue357.py
 ./run_one_test.sh $exe configs/issue239.conf testcases/issue239.py
@@ -12,3 +14,4 @@ cd $(dirname $0)
 ./run_one_test.sh $exe configs/issue314.conf testcases/issue314.py
 ./run_one_test.sh $exe configs/issue314.conf testcases/issue314_2.py
 ./run_one_test.sh $exe configs/issue314.conf testcases/issue314_3.py
+./run_one_test.sh $exe /dev/null testcases/issue299.py

tests/testcases/issue299.py

@@ -0,0 +1,112 @@
+#!/usr/bin/env python3
+
+import xcffib.xproto as xproto
+import xcffib
+import time
+import os
+import subprocess
+import asyncio
+from dbus_next.aio import MessageBus
+from dbus_next.message import Message, MessageType
+from common import *
+
+display = os.environ["DISPLAY"].replace(":", "_")
+conn = xcffib.connect()
+setup = conn.get_setup()
+root = setup.roots[0].root
+visual = setup.roots[0].root_visual
+depth = setup.roots[0].root_depth
+x = xproto.xprotoExtension(conn)
+visual32 = find_32bit_visual(conn)
+
+async def get_client_win_async(wid):
+    message = await bus.call(Message(destination='com.github.chjj.compton.'+display,
+        path='/',
+        interface='com.github.chjj.compton',
+        member='win_get',
+        signature='us',
+        body=[wid, 'client_win']))
+    return message.body[0]
+
+def get_client_win(wid):
+    return loop.run_until_complete(get_client_win_async(wid))
+
+def wait():
+    time.sleep(0.5)
+
+def create_client_window(name):
+    client_win = conn.generate_id()
+    print("Window : ", hex(client_win))
+    conn.core.CreateWindowChecked(depth, client_win, root, 0, 0, 100, 100, 0,
+        xproto.WindowClass.InputOutput, visual, 0, []).check()
+    set_window_name(conn, client_win, "Test window "+name)
+    set_window_class(conn, client_win, "Test windows")
+    set_window_state(conn, client_win, 1)
+    conn.core.MapWindowChecked(client_win).check()
+    return client_win
+
+loop = asyncio.get_event_loop()
+bus = loop.run_until_complete(MessageBus().connect())
+
+cmid = conn.generate_id()
+colormap = conn.core.CreateColormapChecked(xproto.ColormapAlloc._None, cmid, root, visual32).check()
+
+# Create window
+client_wins = []
+for i in range(0,2):
+    client_wins.append(create_client_window(str(i)))
+
+# Create frame window
+frame_win = conn.generate_id()
+print("Window : ", hex(frame_win))
+conn.core.CreateWindowChecked(depth, frame_win, root, 0, 0, 200, 200, 0,
+    xproto.WindowClass.InputOutput, visual, 0, []).check()
+set_window_name(conn, frame_win, "Frame")
+conn.core.MapWindowChecked(frame_win).check()
+
+# Scenario 1.1
+# 1. reparent placeholder to frame
+conn.core.ReparentWindowChecked(client_wins[0], frame_win, 0, 0).check()
+wait()
+# 2. reparent real client to frame
+conn.core.ReparentWindowChecked(client_wins[1], frame_win, 0, 0).check()
+wait()
+# 3. detach the placeholder
+conn.core.ReparentWindowChecked(client_wins[0], root, 0, 0).check()
+wait()
+assert get_client_win(frame_win) == client_wins[1]
+
+# Scenario 1.2
+# 1. reparent placeholder to frame
+conn.core.ReparentWindowChecked(client_wins[0], frame_win, 0, 0).check()
+wait()
+# 2. reparent real client to frame
+conn.core.ReparentWindowChecked(client_wins[1], frame_win, 0, 0).check()
+wait()
+# 3. destroy the placeholder
+conn.core.DestroyWindowChecked(client_wins[0]).check()
+wait()
+assert get_client_win(frame_win) == client_wins[1]
+
+client_wins[0] = create_client_window("0")
+
+# Scenario 2
+# 1. frame is unmapped
+conn.core.UnmapWindowChecked(frame_win).check()
+wait()
+# 2. reparent placeholder to frame
+conn.core.ReparentWindowChecked(client_wins[0], frame_win, 0, 0).check()
+wait()
+# 3. destroy placeholder, map frame and reparent real client to frame
+conn.core.DestroyWindowChecked(client_wins[0]).check()
+conn.core.MapWindowChecked(frame_win).check()
+conn.core.ReparentWindowChecked(client_wins[1], frame_win, 0, 0).check()
+wait()
+assert get_client_win(frame_win) == client_wins[1]
+
+client_wins[0] = create_client_window("0")
+
+# Destroy the windows
+for wid in client_wins:
+    conn.core.DestroyWindowChecked(wid).check()
+conn.core.DestroyWindowChecked(frame_win).check()