menu: dynamically generate top-level pipemenus

This eliminates code duplications and aligns the behavior of top-level
pipemenus with sub-pipemenus.

include/menu/menu.h

@@ -20,8 +20,6 @@ enum menuitem_type {
 
 struct menuitem {
 	struct wl_list actions;
-	char *execute;
-	char *id; /* needed for pipemenus */
 	char *text;
 	char *icon_name;
 	const char *arrow;
@@ -33,7 +31,6 @@ struct menuitem {
 	struct wlr_scene_tree *tree;
 	struct wlr_scene_tree *normal_tree;
 	struct wlr_scene_tree *selected_tree;
-	struct menu_pipe_context *pipe_ctx;
 	struct view *client_list_view;  /* used by internal client-list */
 	struct wl_list link; /* menu.menuitems */
 };
@@ -43,6 +40,7 @@ struct menu {
 	char *id;
 	char *label;
 	char *icon_name;
+	char *execute;
 	struct menu *parent;
 	struct menu_pipe_context *pipe_ctx;
 
@@ -57,7 +55,7 @@ struct menu {
 		struct menuitem *item;
 	} selection;
 	struct wlr_scene_tree *scene_tree;
-	bool is_pipemenu;
+	bool is_pipemenu_child;
 	bool align_left;
 	bool has_icons;
 

src/menu/menu.c

@@ -49,9 +49,8 @@ static bool waiting_for_pipe_menu;
 static struct menuitem *selected_item;
 
 struct menu_pipe_context {
-	struct server *server;
+	struct wlr_box anchor_rect;
-	struct menuitem *item;
+	struct menu *pipemenu;
-	struct menu *top_level_menu;
 	struct buf buf;
 	struct wl_event_source *event_read;
 	struct wl_event_source *event_timeout;
@@ -88,8 +87,7 @@ menu_create(struct server *server, const char *id, const char *label)
 	menu->label = xstrdup(label ? label : id);
 	menu->parent = current_menu;
 	menu->server = server;
-	menu->is_pipemenu = waiting_for_pipe_menu;
+	menu->is_pipemenu_child = waiting_for_pipe_menu;
-	menu->size.width = server->theme->menu_min_width;
 	return menu;
 }
 
@@ -481,16 +479,11 @@ fill_item(char *nodename, char *content)
 static void
 item_destroy(struct menuitem *item)
 {
-	if (item->pipe_ctx) {
-		item->pipe_ctx->item = NULL;
-	}
 	wl_list_remove(&item->link);
 	action_list_free(&item->actions);
 	if (item->tree) {
 		wlr_scene_node_destroy(&item->tree->node);
 	}
-	free(item->execute);
-	free(item->id);
 	free(item->text);
 	free(item->icon_name);
 	free(item);
@@ -636,32 +629,6 @@ static bool parse_buf(struct server *server, struct buf *buf);
 static int handle_pipemenu_readable(int fd, uint32_t mask, void *_ctx);
 static int handle_pipemenu_timeout(void *_ctx);
 
-static void
-parse_root_pipemenu(struct menu *top_level_menu, const char *execute)
-{
-	int pipe_fd = 0;
-	pid_t pid = spawn_piped(execute, &pipe_fd);
-	if (pid <= 0) {
-		wlr_log(WLR_ERROR, "Failed to spawn pipe menu process %s", execute);
-		return;
-	}
-
-	struct menu_pipe_context *ctx = znew(*ctx);
-	ctx->server = top_level_menu->server;
-	ctx->top_level_menu = top_level_menu;
-	ctx->pid = pid;
-	ctx->pipe_fd = pipe_fd;
-	ctx->buf = BUF_INIT;
-	top_level_menu->pipe_ctx = ctx;
-
-	ctx->event_read = wl_event_loop_add_fd(ctx->server->wl_event_loop,
-		pipe_fd, WL_EVENT_READABLE, handle_pipemenu_readable, ctx);
-
-	ctx->event_timeout = wl_event_loop_add_timer(ctx->server->wl_event_loop,
-		handle_pipemenu_timeout, ctx);
-	wl_event_source_timer_update(ctx->event_timeout, PIPEMENU_TIMEOUT_IN_MS);
-}
-
 /*
  * <menu> elements have three different roles:
  *  * Definition of (sub)menu - has ID, LABEL and CONTENT
@@ -678,24 +645,24 @@ handle_menu_element(xmlNode *n, struct server *server)
 
 	if (execute && label && id) {
 		wlr_log(WLR_DEBUG, "pipemenu '%s:%s:%s'", id, label, execute);
+
+		struct menu *pipemenu = menu_create(server, id, label);
+		pipemenu->execute = xstrdup(execute);
 		if (!current_menu) {
 			/*
-			 * Handle pipemenu as the root-menu such this:
+			 * A pipemenu may not have its parent like:
 			 *
 			 * <?xml version="1.0" encoding="UTF-8"?>
 			 * <openbox_menu>
 			 *   <menu id="root-menu" label="foo" execute="bar"/>
 			 * </openbox_menu>
 			 */
-			struct menu *menu = menu_create(server, id, label);
-			parse_root_pipemenu(menu, execute);
 		} else {
 			current_item = item_create(current_menu, label,
 				/* arrow */ true);
 			fill_item("icon", icon_name);
 			current_item_action = NULL;
-			current_item->execute = xstrdup(execute);
+			current_item->submenu = pipemenu;
-			current_item->id = xstrdup(id);
 		}
 	} else if ((label && id) || is_toplevel_static_menu_definition(n, id)) {
 		/*
@@ -749,7 +716,7 @@ handle_menu_element(xmlNode *n, struct server *server)
 		 * pipemenu opening the "root-menu" or similar.
 		 */
 
-		if (current_menu && current_menu->is_pipemenu) {
+		if (waiting_for_pipe_menu) {
 			wlr_log(WLR_ERROR,
 				"cannot link to static menu from pipemenu");
 			goto error;
@@ -1178,6 +1145,8 @@ nullify_item_pointing_to_this_menu(struct menu *menu)
 	}
 }
 
+static void pipemenu_ctx_destroy(struct menu_pipe_context *ctx);
+
 static void
 menu_free(struct menu *menu)
 {
@@ -1194,7 +1163,8 @@ menu_free(struct menu *menu)
 	}
 
 	if (menu->pipe_ctx) {
-		menu->pipe_ctx->top_level_menu = NULL;
+		pipemenu_ctx_destroy(menu->pipe_ctx);
+		assert(!menu->pipe_ctx);
 	}
 
 	/*
@@ -1208,6 +1178,7 @@ menu_free(struct menu *menu)
 	zfree(menu->id);
 	zfree(menu->label);
 	zfree(menu->icon_name);
+	zfree(menu->execute);
 	zfree(menu);
 }
 
@@ -1281,15 +1252,22 @@ menu_set_selection(struct menu *menu, struct menuitem *item)
  * item may be selected multiple times.
  */
 static void
-destroy_pipemenus(struct server *server)
+reset_pipemenus(struct server *server)
 {
 	wlr_log(WLR_DEBUG, "number of menus before close=%d",
 		wl_list_length(&server->menus));
 
 	struct menu *iter, *tmp;
 	wl_list_for_each_safe(iter, tmp, &server->menus, link) {
-		if (iter->is_pipemenu) {
+		if (iter->is_pipemenu_child) {
+			/* Destroy submenus of pipemenus */
 			menu_free(iter);
+		} else if (iter->execute) {
+			/*
+			 * Destroy items and scene-nodes of pipemenus so that
+			 * they are generated again when being opened
+			 */
+			reset_menu(iter);
 		}
 	}
 
@@ -1300,12 +1278,18 @@ destroy_pipemenus(struct server *server)
 static void
 _close(struct menu *menu)
 {
-	wlr_scene_node_set_enabled(&menu->scene_tree->node, false);
+	if (menu->scene_tree) {
+		wlr_scene_node_set_enabled(&menu->scene_tree->node, false);
+	}
 	menu_set_selection(menu, NULL);
 	if (menu->selection.menu) {
 		_close(menu->selection.menu);
 		menu->selection.menu = NULL;
 	}
+	if (menu->pipe_ctx) {
+		pipemenu_ctx_destroy(menu->pipe_ctx);
+		assert(!menu->pipe_ctx);
+	}
 }
 
 static void
@@ -1335,6 +1319,8 @@ open_menu(struct menu *menu, struct wlr_box anchor_rect)
 	wlr_scene_node_set_enabled(&menu->scene_tree->node, true);
 }
 
+static void open_pipemenu_async(struct menu *pipemenu, struct wlr_box anchor_rect);
+
 void
 menu_open_root(struct menu *menu, int x, int y)
 {
@@ -1346,7 +1332,13 @@ menu_open_root(struct menu *menu, int x, int y)
 
 	assert(!menu->server->menu_current);
 
-	open_menu(menu, (struct wlr_box){.x = x, .y = y});
+	struct wlr_box anchor_rect = {.x = x, .y = y};
+	if (menu->execute) {
+		open_pipemenu_async(menu, anchor_rect);
+	} else {
+		open_menu(menu, anchor_rect);
+	}
+
 	menu->server->menu_current = menu;
 	selected_item = NULL;
 	seat_focus_override_begin(&menu->server->seat,
@@ -1356,81 +1348,21 @@ menu_open_root(struct menu *menu, int x, int y)
 static void
 create_pipe_menu(struct menu_pipe_context *ctx)
 {
-	if (ctx->top_level_menu) {
+	struct server *server = ctx->pipemenu->server;
-		/*
+	struct menu *old_current_menu = current_menu;
-		 * We execute the scripts for the toplevel pipemenus at startup
-		 * or Reconfigure, but they can be opened before they finish
-		 * execution, usually with their content empty. Make sure they
-		 * are closed and emptied.
-		 */
-		if (ctx->server->menu_current == ctx->top_level_menu) {
-			menu_close_root(ctx->server);
-		}
-		struct menuitem *item, *tmp;
-		wl_list_for_each_safe(item, tmp, &ctx->top_level_menu->menuitems, link) {
-			item_destroy(item);
-		}
-
-		menu_level++;
-		current_menu = ctx->top_level_menu;
-		if (!parse_buf(ctx->server, &ctx->buf)) {
-			wlr_log(WLR_ERROR, "Failed to parse piped top level menu %s",
-				ctx->top_level_menu->id);
-		}
-		menu_level--;
-		validate(ctx->server);
-		return;
-	}
-
-	assert(ctx->item);
-
-	struct menu *pipe_parent = ctx->item->parent;
-	if (!pipe_parent) {
-		wlr_log(WLR_INFO, "[pipemenu %ld] invalid parent",
-			(long)ctx->pid);
-		return;
-	}
-	if (!pipe_parent->scene_tree->node.enabled) {
-		wlr_log(WLR_INFO, "[pipemenu %ld] parent menu already closed",
-			(long)ctx->pid);
-		return;
-	}
-
-	/*
-	 * Pipemenus do not contain a toplevel <menu> element so we have to
-	 * create that first `struct menu`.
-	 */
-	struct menu *pipe_menu = menu_create(ctx->server, ctx->item->id, /*label*/ NULL);
-	pipe_menu->is_pipemenu = true;
-	pipe_menu->triggered_by_view = pipe_parent->triggered_by_view;
-	pipe_menu->parent = pipe_parent;
-
 	menu_level++;
-	current_menu = pipe_menu;
+	current_menu = ctx->pipemenu;
-	if (!parse_buf(ctx->server, &ctx->buf)) {
+	if (!parse_buf(server, &ctx->buf)) {
-		menu_free(pipe_menu);
-		ctx->item->submenu = NULL;
 		goto restore_menus;
 	}
-	ctx->item->submenu = pipe_menu;
+	/* TODO: apply validate() only for generated pipemenus */
-
+	validate(server);
-	/*
-	 * TODO: refactor validate() and post_processing() to only
-	 * operate from current point onwards
-	 */
-
-	struct wlr_box anchor_rect =
-		get_item_anchor_rect(ctx->server->theme, ctx->item);
-	open_menu(pipe_menu, anchor_rect);
-
-	validate(ctx->server);
 
 	/* Finally open the new submenu tree */
-	wlr_scene_node_set_enabled(&pipe_menu->scene_tree->node, true);
+	open_menu(ctx->pipemenu, ctx->anchor_rect);
-	pipe_parent->selection.menu = pipe_menu;
 
 restore_menus:
-	current_menu = pipe_parent;
+	current_menu = old_current_menu;
 	menu_level--;
 }
 
@@ -1441,11 +1373,8 @@ pipemenu_ctx_destroy(struct menu_pipe_context *ctx)
 	wl_event_source_remove(ctx->event_timeout);
 	spawn_piped_close(ctx->pid, ctx->pipe_fd);
 	buf_reset(&ctx->buf);
-	if (ctx->item) {
+	if (ctx->pipemenu) {
-		ctx->item->pipe_ctx = NULL;
+		ctx->pipemenu->pipe_ctx = NULL;
-	}
-	if (ctx->top_level_menu) {
-		ctx->top_level_menu->pipe_ctx = NULL;
 	}
 	free(ctx);
 	waiting_for_pipe_menu = false;
@@ -1456,7 +1385,7 @@ handle_pipemenu_timeout(void *_ctx)
 {
 	struct menu_pipe_context *ctx = _ctx;
 	wlr_log(WLR_ERROR, "[pipemenu %ld] timeout reached, killing %s",
-		(long)ctx->pid, ctx->item ? ctx->item->execute : "n/a");
+		(long)ctx->pid, ctx->pipemenu->execute);
 	kill(ctx->pid, SIGTERM);
 	pipemenu_ctx_destroy(ctx);
 	return 0;
@@ -1470,14 +1399,6 @@ handle_pipemenu_readable(int fd, uint32_t mask, void *_ctx)
 	char data[8193];
 	ssize_t size;
 
-	if (!ctx->item && !ctx->top_level_menu) {
-		/* parent menu item got destroyed in the meantime */
-		wlr_log(WLR_INFO, "[pipemenu %ld] parent menu item destroyed",
-			(long)ctx->pid);
-		kill(ctx->pid, SIGTERM);
-		goto clean_up;
-	}
-
 	do {
 		/* leave space for terminating NULL byte */
 		size = read(fd, data, sizeof(data) - 1);
@@ -1485,7 +1406,7 @@ handle_pipemenu_readable(int fd, uint32_t mask, void *_ctx)
 
 	if (size == -1) {
 		wlr_log_errno(WLR_ERROR, "[pipemenu %ld] failed to read data (%s)",
-			(long)ctx->pid, ctx->item ? ctx->item->execute : "n/a");
+			(long)ctx->pid, ctx->pipemenu->execute);
 		goto clean_up;
 	}
 
@@ -1493,7 +1414,7 @@ handle_pipemenu_readable(int fd, uint32_t mask, void *_ctx)
 	if (ctx->buf.len + size > PIPEMENU_MAX_BUF_SIZE) {
 		wlr_log(WLR_ERROR, "[pipemenu %ld] too big (> %d bytes); killing %s",
 			(long)ctx->pid, PIPEMENU_MAX_BUF_SIZE,
-			ctx->item ? ctx->item->execute : "n/a");
+			ctx->pipemenu->execute);
 		kill(ctx->pid, SIGTERM);
 		goto clean_up;
 	}
@@ -1519,42 +1440,39 @@ clean_up:
 }
 
 static void
-parse_pipemenu(struct menuitem *item)
+open_pipemenu_async(struct menu *pipemenu, struct wlr_box anchor_rect)
 {
-	if (!is_unique_id(item->parent->server, item->id)) {
+	struct server *server = pipemenu->server;
-		wlr_log(WLR_ERROR, "duplicate id '%s'; abort pipemenu", item->id);
-		return;
-	}
 
-	if (item->pipe_ctx) {
+	assert(!pipemenu->pipe_ctx);
-		wlr_log(WLR_ERROR, "item already has a pipe context attached");
+	assert(!pipemenu->scene_tree);
-		return;
-	}
 
 	int pipe_fd = 0;
-	pid_t pid = spawn_piped(item->execute, &pipe_fd);
+	pid_t pid = spawn_piped(pipemenu->execute, &pipe_fd);
 	if (pid <= 0) {
-		wlr_log(WLR_ERROR, "Failed to spawn pipe menu process %s", item->execute);
+		wlr_log(WLR_ERROR, "Failed to spawn pipe menu process %s",
+			pipemenu->execute);
 		return;
 	}
 
 	waiting_for_pipe_menu = true;
 	struct menu_pipe_context *ctx = znew(*ctx);
-	ctx->server = item->parent->server;
-	ctx->item = item;
 	ctx->pid = pid;
 	ctx->pipe_fd = pipe_fd;
 	ctx->buf = BUF_INIT;
-	item->pipe_ctx = ctx;
+	ctx->anchor_rect = anchor_rect;
+	ctx->pipemenu = pipemenu;
+	pipemenu->pipe_ctx = ctx;
 
-	ctx->event_read = wl_event_loop_add_fd(ctx->server->wl_event_loop,
+	ctx->event_read = wl_event_loop_add_fd(server->wl_event_loop,
 		pipe_fd, WL_EVENT_READABLE, handle_pipemenu_readable, ctx);
 
-	ctx->event_timeout = wl_event_loop_add_timer(ctx->server->wl_event_loop,
+	ctx->event_timeout = wl_event_loop_add_timer(server->wl_event_loop,
 		handle_pipemenu_timeout, ctx);
 	wl_event_source_timer_update(ctx->event_timeout, PIPEMENU_TIMEOUT_IN_MS);
 
-	wlr_log(WLR_DEBUG, "[pipemenu %ld] executed: %s", (long)ctx->pid, ctx->item->execute);
+	wlr_log(WLR_DEBUG, "[pipemenu %ld] executed: %s",
+		(long)ctx->pid, ctx->pipemenu->execute);
 }
 
 static void
@@ -1583,13 +1501,6 @@ menu_process_item_selection(struct menuitem *item)
 		menu_close(item->parent->selection.menu);
 	}
 
-	/* Pipemenu */
-	if (item->execute && !item->submenu) {
-		/* pipemenus are generated async */
-		parse_pipemenu(item);
-		return;
-	}
-
 	if (item->submenu) {
 		/* Sync the triggering view */
 		item->submenu->triggered_by_view = item->parent->triggered_by_view;
@@ -1598,7 +1509,11 @@ menu_process_item_selection(struct menuitem *item)
 		/* And open the new submenu tree */
 		struct wlr_box anchor_rect =
 			get_item_anchor_rect(item->submenu->server->theme, item);
-		open_menu(item->submenu, anchor_rect);
+		if (item->submenu->execute && !item->submenu->scene_tree) {
+			open_pipemenu_async(item->submenu, anchor_rect);
+		} else {
+			open_menu(item->submenu, anchor_rect);
+		}
 	}
 
 	item->parent->selection.menu = item->submenu;
@@ -1684,7 +1599,7 @@ menu_execute_item(struct menuitem *item)
 				&item->actions, NULL);
 	}
 
-	destroy_pipemenus(server);
+	reset_pipemenus(server);
 	return true;
 }
 
@@ -1773,7 +1688,7 @@ menu_close_root(struct server *server)
 
 	menu_close(server->menu_current);
 	server->menu_current = NULL;
-	destroy_pipemenus(server);
+	reset_pipemenus(server);
 	seat_focus_override_end(&server->seat);
 }