wip

meson.build

@@ -68,7 +68,7 @@ if get_option('warning_level') != '0'
             'sign-compare', 'type-limits', 'uninitialized', 'shift-negative-value',
             'unused-but-set-parameter', 'unused-parameter', 'implicit-fallthrough=2',
             'no-unknown-warning-option', 'no-missing-braces', 'conversion', 'empty-body',
-            'no-c2x-extensions' ]
+            'no-c2x-extensions', 'no-discarded-qualifiers']
   bad_warns_ubsan = [
     'no-format-overflow' # see gcc bug 87884, enabling UBSAN makes gcc spit out spurious warnings
                          # (if you saw this comment, went and checked gcc bugzilla, and found out

src/event.c

@@ -281,7 +281,7 @@ static inline void ev_configure_notify(session_t *ps, xcb_configure_notify_event
 	          ev->event, ev->window, ev->above_sibling, ev->override_redirect);
 	if (ev->window == ps->c.screen_info->root) {
 		set_root_flags(ps, ROOT_FLAGS_CONFIGURED);
-	} else {
+	} else if (!wm_is_wid_masked(ps->wm, ev->event)) {
 		configure_win(ps, ev);
 	}
 }
@@ -307,6 +307,10 @@ static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) {
 		return;
 	}
 
+	if (wm_is_wid_masked(ps->wm, ev->event)) {
+		return;
+	}
+
 	auto cursor = wm_find(ps->wm, ev->window);
 	if (cursor == NULL) {
 		log_error("Map event received for unknown window %#010x, overlay is "
@@ -338,6 +342,10 @@ static inline void ev_unmap_notify(session_t *ps, xcb_unmap_notify_event_t *ev)
 		return;
 	}
 
+	if (wm_is_wid_masked(ps->wm, ev->event)) {
+		return;
+	}
+
 	auto cursor = wm_find(ps->wm, ev->window);
 	if (cursor == NULL) {
 		log_error("Unmap event received for unknown window %#010x", ev->window);
@@ -352,25 +360,14 @@ static inline void ev_unmap_notify(session_t *ps, xcb_unmap_notify_event_t *ev)
 static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t *ev) {
 	log_debug("Window %#010x has new parent: %#010x, override_redirect: %d",
 	          ev->window, ev->parent, ev->override_redirect);
+	wm_reparent(ps->wm, ev->window, ev->parent);
+}
 
-	auto cursor = wm_find(ps->wm, ev->window);
-	if (cursor == NULL) {
-		log_error("Reparent event received for unknown window %#010x", ev->window);
-		return;
-	}
-	auto new_parent = wm_find(ps->wm, ev->parent);
-	if (new_parent == NULL) {
-		log_error("Reparent event received for window %#010x, but its new parent "
-		          "window %#010x is not in our tree, ignoring. Expect "
-		          "malfunction.",
-		          ev->window, ev->parent);
+static inline void ev_circulate_notify(session_t *ps, xcb_circulate_notify_event_t *ev) {
+	if (wm_is_wid_masked(ps->wm, ev->event)) {
 		return;
 	}
 
-	wm_reparent(ps->wm, cursor, new_parent);
-}
-
-static inline void ev_circulate_notify(session_t *ps, xcb_circulate_notify_event_t *ev) {
 	auto cursor = wm_find(ps->wm, ev->window);
 
 	if (cursor == NULL) {
@@ -446,6 +443,10 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t
 		return;
 	}
 
+	if (wm_is_wid_masked(ps->wm, ev->window)) {
+		return;
+	}
+
 	ps->pending_updates = true;
 	auto cursor = wm_find(ps->wm, ev->window);
 	if (cursor == NULL) {

src/wm/tree.c

@@ -254,6 +254,7 @@ wm_tree_new_window(struct wm_tree *tree, xcb_window_t id, struct wm_tree_node *p
 			wm_tree_enqueue_change(tree, (struct wm_tree_change){
 			                                 .toplevel = node->id,
 			                                 .type = WM_TREE_CHANGE_TOPLEVEL_NEW,
+											 .new_ = node,
 			                             });
 		}
 	}
@@ -267,10 +268,11 @@ wm_tree_refresh_client_and_queue_change(struct wm_tree *tree, struct wm_tree_nod
 	BUG_ON(toplevel->parent->parent != NULL);
 	auto new_client = wm_tree_find_client(toplevel);
 	if (new_client != toplevel->client_window) {
-		struct wm_tree_change change = {
-		    .toplevel = toplevel->id,
-		    .type = WM_TREE_CHANGE_CLIENT,
-		    .client = {.old = WM_TREEID_NONE, .new_ = WM_TREEID_NONE}};
+		struct wm_tree_change change = {.toplevel = toplevel->id,
+		                                .type = WM_TREE_CHANGE_CLIENT,
+		                                .client = {.toplevel = toplevel,
+		                                           .old = WM_TREEID_NONE,
+		                                           .new_ = WM_TREEID_NONE}};
 		if (toplevel->client_window != NULL) {
 			change.client.old = toplevel->client_window->id;
 		}
@@ -406,6 +408,7 @@ void wm_tree_reparent(struct wm_tree *tree, struct wm_tree_node *node,
 		wm_tree_enqueue_change(tree, (struct wm_tree_change){
 		                                 .toplevel = node->id,
 		                                 .type = WM_TREE_CHANGE_TOPLEVEL_NEW,
+										 .new_ = node,
 		                             });
 	} else {
 		wm_tree_refresh_client_and_queue_change(tree, toplevel);

src/wm/wm.c

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MPL-2.0
 // Copyright (c) Yuxuan Shui
 
+#include <stddef.h>
 #include <uthash.h>
 #include <xcb/xproto.h>
 
@@ -31,6 +32,9 @@ struct wm {
 	/// Incomplete imports. See `wm_import_incomplete` for an explanation.
 	/// This is a dynarr.
 	struct wm_tree_node **incompletes;
+	/// Tree nodes that we have chosen to forget, but we might still receive some
+	/// events from, we keep them here to ignore those events.
+	struct wm_tree_node **masked;
 };
 
 // TODO(yshui): this is a bit weird and I am not decided on it yet myself. Maybe we can
@@ -56,6 +60,19 @@ static inline struct wm_tree_node *to_tree_node_mut(struct wm_ref *cursor) {
 	                      : NULL;
 }
 
+static ptrdiff_t wm_find_masked(struct wm *wm, xcb_window_t wid) {
+	dynarr_foreach(wm->masked, m) {
+		if ((*m)->id.x == wid) {
+			return m - wm->masked;
+		}
+	}
+	return -1;
+}
+
+bool wm_is_wid_masked(struct wm *wm, xcb_window_t wid) {
+	return wm_find_masked(wm, wid) != -1;
+}
+
 struct win *wm_ref_deref(const struct wm_ref *cursor) {
 	return to_tree_node(cursor)->win;
 }
@@ -160,6 +177,7 @@ struct wm *wm_new(void) {
 	auto wm = ccalloc(1, struct wm);
 	wm_tree_init(&wm->tree);
 	wm->incompletes = dynarr_new(struct wm_tree_node *, 4);
+	wm->masked = dynarr_new(struct wm_tree_node *, 8);
 	return wm;
 }
 
@@ -186,6 +204,12 @@ void wm_free(struct wm *wm) {
 }
 
 void wm_destroy(struct wm *wm, xcb_window_t wid) {
+	auto masked = wm_find_masked(wm, wid);
+	if (masked != -1) {
+		dynarr_remove_swap(wm->masked, (size_t)masked);
+		return;
+	}
+
 	struct wm_tree_node *node = wm_tree_find(&wm->tree, wid);
 	if (!node) {
 		log_error("Trying to destroy window %#010x, but it's not in the tree. "
@@ -206,19 +230,60 @@ void wm_reap_zombie(struct wm_ref *zombie) {
 	wm_tree_reap_zombie(to_tree_node_mut(zombie));
 }
 
-void wm_reparent(struct wm *wm, struct wm_ref *cursor, struct wm_ref *new_parent) {
-	wm_tree_reparent(&wm->tree, to_tree_node_mut(cursor), to_tree_node_mut(new_parent));
+void wm_reparent(struct wm *wm, xcb_window_t wid, xcb_window_t parent) {
+	auto window = wm_tree_find(&wm->tree, wid);
+	auto new_parent = wm_tree_find(&wm->tree, parent);
+	// We delete the window here if parent is not found, or if the parent is
+	// an incomplete import. We will rediscover this window later in
+	// `wm_complete_import`. Keeping it around will only confuse us.
+	bool should_forget =
+	    new_parent == NULL || dynarr_find_pod(wm->incompletes, new_parent) != -1;
+	if (window == NULL) {
+		log_debug("Reparenting window %#010x which is not in our tree. Assuming "
+		          "it came from fog of war.",
+		          wid);
+		if (!should_forget) {
+			wm_import_incomplete(wm, wid, parent);
+		}
+		return;
+	}
+	if (should_forget) {
+		log_debug("Window %#010x reparented to window %#010x which is "
+		          "%s, forgetting and masking instead.",
+		          window->id.x, parent,
+		          new_parent == NULL ? "not in our tree" : "an incomplete import");
+
+		wm_tree_detach(&wm->tree, window);
+		for (auto curr = window; curr != NULL; curr = wm_tree_next(curr, window)) {
+			HASH_DEL(wm->tree.nodes, curr);
+			dynarr_push(wm->masked, curr);
+		}
+	} else {
+		wm_tree_reparent(&wm->tree, window, new_parent);
+	}
 }
 
 void wm_set_has_wm_state(struct wm *wm, struct wm_ref *cursor, bool has_wm_state) {
 	wm_tree_set_wm_state(&wm->tree, to_tree_node_mut(cursor), has_wm_state);
 }
 
 void wm_import_incomplete(struct wm *wm, xcb_window_t wid, xcb_window_t parent) {
+	auto masked = wm_find_masked(wm, wid);
+	if (masked != -1) {
+		// A new window created with the same wid means the window we chose to
+		// forget has been deleted (without us knowing), and its ID was then
+		// reused.
+		dynarr_remove_swap(wm->masked, (size_t)masked);
+	}
 	auto parent_cursor = NULL;
 	if (parent != XCB_NONE) {
 		parent_cursor = wm_tree_find(&wm->tree, parent);
-		BUG_ON_NULL(parent_cursor);
+		if (parent_cursor == NULL) {
+			log_error("Importing window %#010x, but its parent %#010x is not "
+			          "in our tree, ignoring. Expect malfunction.",
+			          wid, parent);
+			return;
+		}
 	}
 	log_debug("Importing window %#010x with parent %#010x", wid, parent);
 	auto new = wm_tree_new_window(&wm->tree, wid, parent_cursor);
@@ -267,48 +332,36 @@ static void wm_complete_import_subtree(struct wm *wm, struct x_connection *c,
 			// means the windows will naturally be in the correct stacking
 			// order.
 			auto existing = wm_tree_find(&wm->tree, children[i]);
-			bool is_incomplete = false;
 			if (existing != NULL) {
-				// We might have already known about this window, but that
-				// doesn't mean we have setup, for example, event masks
-				// for it, if it's on the incomplete list. And if it is,
-				// we also need to remove it.
+				// This should never happen: we haven't subscribed to
+				// child creation events yet, and any window reparented to
+				// an incomplete is deleted. report an error and try to
+				// recover.
 				auto index = dynarr_find_pod(wm->incompletes, existing);
-				is_incomplete = index != -1;
-				if (is_incomplete) {
+				if (index != -1) {
 					dynarr_remove_swap(wm->incompletes, (size_t)index);
 				}
-
-				xcb_window_t parent_id = existing->parent != NULL
-				                             ? existing->parent->id.x
-				                             : XCB_NONE;
-				if (existing->parent != curr) {
-					log_error("X says %#010x is a child of %#010x, "
-					          "but according to us it's a child of "
-					          "%#010x. Trying to recover, but expect "
-					          "malfunction.",
-					          children[i], curr->id.x, parent_id);
-					wm_tree_reparent(&wm->tree, existing, curr);
-				} else {
-					log_debug("We already know about window %#010x, "
-					          "moving it to the top.",
-					          children[i]);
-					wm_tree_move_to_end(&wm->tree, existing, false);
-				}
-			} else {
-				is_incomplete = true;
-				existing = wm_tree_new_window(&wm->tree, children[i], curr);
-			}
-
-			if (is_incomplete) {
-				wm_complete_import_single(wm, c, atoms, existing);
+				log_error("Window %#010x already exists in the tree, but "
+				          "it appeared again as a "
+				          "child of window %#010x. Deleting the old one, "
+				          "but expect malfunction.",
+				          children[i], curr->id.x);
+				wm_tree_destroy_window(&wm->tree, existing);
 			}
+			existing = wm_tree_new_window(&wm->tree, children[i], curr);
+			wm_complete_import_single(wm, c, atoms, existing);
 		}
 		free(tree);
 	}
 }
 
 void wm_complete_import(struct wm *wm, struct x_connection *c, struct atom *atoms) {
+	// Unveil the fog of war
+	dynarr_foreach(wm->masked, m) {
+		free(*m);
+	}
+	dynarr_clear_pod(wm->masked);
+
 	while (!dynarr_is_empty(wm->incompletes)) {
 		auto i = dynarr_pop(wm->incompletes);
 		// This function modifies `wm->incompletes`, so we can't use

src/wm/wm.h

@@ -149,7 +149,7 @@ bool attr_pure wm_ref_is_zombie(const struct wm_ref *cursor);
 void wm_destroy(struct wm *wm, xcb_window_t wid);
 /// Remove a zombie window from the window tree.
 void wm_reap_zombie(struct wm_ref *zombie);
-void wm_reparent(struct wm *wm, struct wm_ref *cursor, struct wm_ref *new_parent);
+void wm_reparent(struct wm *wm, xcb_window_t wid, xcb_window_t parent);
 void wm_set_has_wm_state(struct wm *wm, struct wm_ref *cursor, bool has_wm_state);
 
 /// Create a tree node for `wid`, with `parent` as its parent. The parent must already
@@ -158,17 +158,33 @@ void wm_set_has_wm_state(struct wm *wm, struct wm_ref *cursor, bool has_wm_state
 /// expectation is that the caller will later call `wm_complete_import` inside the
 /// X critical section to complete the import.
 ///
+/// ## NOTE
+///
 /// The reason for this complicated dance is because we want to catch all windows ever
 /// created on X server's side. For a newly created windows, we will setup a
 /// SubstructureNotify event mask to catch any new windows created under it. But between
 /// the time we received the creation event and the time we setup the event mask, if any
 /// windows were created under the new window, we will miss them. Therefore we have to
 /// scan the new windows in X critical section so they won't change as we scan.
 ///
-/// On the other hand, we can't push everything to the X critical section, because there
-/// can be subsequent events that relate to the new windows, if we don't create something
-/// in the tree immediately, we can't handle those events. That's why this has to be
-/// broken into two steps.
+/// On the other hand, we can't push everything to the X critical section, because updating
+/// the window stack requires knowledge of all windows in the stack. Think ConfigureNotify,
+/// if we don't know about the `above_sibling` window, we don't know where to put the window.
+/// So we need to create an incomplete import first.
+///
+/// But wait, this is actually way more involved. Because we choose to not set up event masks
+/// for incomplete imports (we can also choose otherwise, but that also has its own set of
+/// problems), there is a whole subtree of windows we don't know about. And those windows
+/// might involve in reparent events. To handle that, we essentially put "fog of war" under
+/// any incomplete imports, anything reparented into the fog is lost, and will be rediscovered
+/// later during subtree scans. If a window is reparented out of the fog, then it's treated as
+/// if a brand new window was created.
+///
+/// But wait again, there's more. We can delete "lost" windows on our side and unset event masks,
+/// but again because this is racy, we might still receive some events for those windows. So we
+/// have to keep a list of "lost" windows, and correctly ignore events sent for them. (And since
+/// we are actively ignoring events from these windows, we might as well not unset their event
+/// masks, saves us a trip to the X server).
 ///
 /// (Now you have a glimpse of how much X11 sucks.)
 void wm_import_incomplete(struct wm *wm, xcb_window_t wid, xcb_window_t parent);
@@ -183,4 +199,7 @@ bool wm_has_incomplete_imports(const struct wm *wm);
 /// as well as for the (now completed) placeholder windows.
 void wm_complete_import(struct wm *wm, struct x_connection *c, struct atom *atoms);
 
+
+bool wm_is_wid_masked(struct wm *wm, xcb_window_t wid);
+
 struct wm_change wm_dequeue_change(struct wm *wm);