From cae6fb627f1020b8b4fabaf7e6cb7a7ec7baffff Mon Sep 17 00:00:00 2001 From: Deiz Date: Wed, 3 Oct 2012 21:06:04 -0400 Subject: [PATCH] Improve startup sequence termination conditions If a window with _NET_STARTUP_ID set is moved to another workspace, it will delete any associated startup sequence immediately. This will also occur if a window has a leader with _NET_STARTUP_ID set, if the leader has no container (never been mapped). A startup sequence may also be deleted if it's matched by startup_workspace_for_window() and its 30-second timeout has elapsed. --- include/startup.h | 15 ++++ src/con.c | 32 +++++++ src/startup.c | 112 ++++++++++++++++++------- testcases/t/175-startup-notification.t | 39 ++++++++- 4 files changed, 166 insertions(+), 32 deletions(-) diff --git a/include/startup.h b/include/startup.h index bcc59a0a..e39fe63b 100644 --- a/include/startup.h +++ b/include/startup.h @@ -31,12 +31,27 @@ */ void start_application(const char *command, bool no_startup_id); +/** + * Deletes a startup sequence, ignoring whether its timeout has elapsed. + * Useful when e.g. a window is moved between workspaces and its children + * shouldn't spawn on the original workspace. + * + */ +void startup_sequence_delete(struct Startup_Sequence *sequence); + /** * Called by libstartup-notification when something happens * */ void startup_monitor_event(SnMonitorEvent *event, void *userdata); +/** + * Gets the stored startup sequence for the _NET_STARTUP_ID of a given window. + * + */ +struct Startup_Sequence *startup_sequence_get(i3Window *cwindow, + xcb_get_property_reply_t *startup_id_reply, bool ignore_mapped_leader); + /** * Checks if the given window belongs to a startup notification by checking if * the _NET_STARTUP_ID property is set on the window (or on its leader, if it’s diff --git a/src/con.c b/src/con.c index d872858b..493707d6 100644 --- a/src/con.c +++ b/src/con.c @@ -769,6 +769,38 @@ void con_move_to_workspace(Con *con, Con *workspace, bool fix_coordinates, bool con_focus(con_descend_focused(focus_next)); } + /* If anything within the container is associated with a startup sequence, + * delete it so child windows won't be created on the old workspace. */ + struct Startup_Sequence *sequence; + xcb_get_property_cookie_t cookie; + xcb_get_property_reply_t *startup_id_reply; + + if (!con_is_leaf(con)) { + Con *child; + TAILQ_FOREACH(child, &(con->nodes_head), nodes) { + if (!child->window) + continue; + + cookie = xcb_get_property(conn, false, child->window->id, + A__NET_STARTUP_ID, XCB_GET_PROPERTY_TYPE_ANY, 0, 512); + startup_id_reply = xcb_get_property_reply(conn, cookie, NULL); + + sequence = startup_sequence_get(child->window, startup_id_reply, true); + if (sequence != NULL) + startup_sequence_delete(sequence); + } + } + + if (con->window) { + cookie = xcb_get_property(conn, false, con->window->id, + A__NET_STARTUP_ID, XCB_GET_PROPERTY_TYPE_ANY, 0, 512); + startup_id_reply = xcb_get_property_reply(conn, cookie, NULL); + + sequence = startup_sequence_get(con->window, startup_id_reply, true); + if (sequence != NULL) + startup_sequence_delete(sequence); + } + CALL(parent, on_remove_child); } diff --git a/src/startup.c b/src/startup.c index 89324dbd..ee51664f 100644 --- a/src/startup.c +++ b/src/startup.c @@ -58,7 +58,7 @@ static void startup_timeout(EV_P_ ev_timer *w, int revents) { } /* - * Some applications (such as Firefox) mark a startup sequence as completede + * Some applications (such as Firefox) mark a startup sequence as completed * *before* they even map a window. Therefore, we cannot entirely delete the * startup sequence once it’s marked as complete. Instead, we’ll mark it for * deletion in 30 seconds and use that chance to delete old sequences. @@ -68,15 +68,10 @@ static void startup_timeout(EV_P_ ev_timer *w, int revents) { * the root window cursor. * */ -static int _delete_startup_sequence(struct Startup_Sequence *sequence) { +static int _prune_startup_sequences(void) { time_t current_time = time(NULL); int active_sequences = 0; - /* Mark the given sequence for deletion in 30 seconds. */ - sequence->delete_at = current_time + 30; - DLOG("Will delete startup sequence %s at timestamp %ld\n", - sequence->id, sequence->delete_at); - /* Traverse the list and delete everything which was marked for deletion 30 * seconds ago or earlier. */ struct Startup_Sequence *current, *next; @@ -94,20 +89,35 @@ static int _delete_startup_sequence(struct Startup_Sequence *sequence) { if (current_time <= current->delete_at) continue; - DLOG("Deleting startup sequence %s, delete_at = %ld, current_time = %ld\n", - current->id, current->delete_at, current_time); - - /* Unref the context, will be free()d */ - sn_launcher_context_unref(current->context); - - /* Delete our internal sequence */ - TAILQ_REMOVE(&startup_sequences, current, sequences); + startup_sequence_delete(current); } return active_sequences; } +/** + * Deletes a startup sequence, ignoring whether its timeout has elapsed. + * Useful when e.g. a window is moved between workspaces and its children + * shouldn't spawn on the original workspace. + * + */ +void startup_sequence_delete(struct Startup_Sequence *sequence) { + assert(sequence != NULL); + DLOG("Deleting startup sequence %s, delete_at = %ld, current_time = %ld\n", + sequence->id, sequence->delete_at, time(NULL)); + + /* Unref the context, will be free()d */ + sn_launcher_context_unref(sequence->context); + + /* Delete our internal sequence */ + TAILQ_REMOVE(&startup_sequences, sequence, sequences); + + free(sequence->id); + free(sequence->workspace); + FREE(sequence); +} + /* * Starts the given application by passing it through a shell. We use double fork * to avoid zombie processes. As the started application’s parent exits (immediately), @@ -233,7 +243,13 @@ void startup_monitor_event(SnMonitorEvent *event, void *userdata) { case SN_MONITOR_EVENT_COMPLETED: DLOG("startup sequence %s completed\n", sn_startup_sequence_get_id(snsequence)); - if (_delete_startup_sequence(sequence) == 0) { + /* Mark the given sequence for deletion in 30 seconds. */ + time_t current_time = time(NULL); + sequence->delete_at = current_time + 30; + DLOG("Will delete startup sequence %s at timestamp %ld\n", + sequence->id, sequence->delete_at); + + if (_prune_startup_sequences() == 0) { DLOG("No more startup sequences running, changing root window cursor to default pointer.\n"); /* Change the pointer of the root window to indicate progress */ if (xcursor_supported) @@ -247,32 +263,44 @@ void startup_monitor_event(SnMonitorEvent *event, void *userdata) { } } -/* - * Checks if the given window belongs to a startup notification by checking if - * the _NET_STARTUP_ID property is set on the window (or on its leader, if it’s - * unset). - * - * If so, returns the workspace on which the startup was initiated. - * Returns NULL otherwise. +/** + * Gets the stored startup sequence for the _NET_STARTUP_ID of a given window. * */ -char *startup_workspace_for_window(i3Window *cwindow, xcb_get_property_reply_t *startup_id_reply) { +struct Startup_Sequence *startup_sequence_get(i3Window *cwindow, + xcb_get_property_reply_t *startup_id_reply, bool ignore_mapped_leader) { /* The _NET_STARTUP_ID is only needed during this function, so we get it * here and don’t save it in the 'cwindow'. */ if (startup_id_reply == NULL || xcb_get_property_value_length(startup_id_reply) == 0) { FREE(startup_id_reply); - DLOG("No _NET_STARTUP_ID set on this window\n"); + DLOG("No _NET_STARTUP_ID set on window 0x%08x\n", cwindow->id); if (cwindow->leader == XCB_NONE) return NULL; - xcb_get_property_cookie_t cookie; - cookie = xcb_get_property(conn, false, cwindow->leader, A__NET_STARTUP_ID, XCB_GET_PROPERTY_TYPE_ANY, 0, 512); + /* This is a special case that causes the leader's startup sequence + * to only be returned if it has never been mapped, useful primarily + * when trying to delete a sequence. + * + * It's generally inappropriate to delete a leader's sequence when + * moving a child window, but if the leader has no container, it's + * likely permanently unmapped and the child is the "real" window. */ + if (ignore_mapped_leader && con_by_window_id(cwindow->leader) != NULL) { + DLOG("Ignoring leader window 0x%08x\n", cwindow->leader); + return NULL; + } + DLOG("Checking leader window 0x%08x\n", cwindow->leader); + + xcb_get_property_cookie_t cookie; + + cookie = xcb_get_property(conn, false, cwindow->leader, + A__NET_STARTUP_ID, XCB_GET_PROPERTY_TYPE_ANY, 0, 512); startup_id_reply = xcb_get_property_reply(conn, cookie, NULL); - if (startup_id_reply == NULL || xcb_get_property_value_length(startup_id_reply) == 0) { - DLOG("No _NET_STARTUP_ID set on the leader either\n"); + if (startup_id_reply == NULL || + xcb_get_property_value_length(startup_id_reply) == 0) { FREE(startup_id_reply); + DLOG("No _NET_STARTUP_ID set on the leader either\n"); return NULL; } } @@ -304,5 +332,31 @@ char *startup_workspace_for_window(i3Window *cwindow, xcb_get_property_reply_t * free(startup_id); free(startup_id_reply); + + return sequence; +} + +/* + * Checks if the given window belongs to a startup notification by checking if + * the _NET_STARTUP_ID property is set on the window (or on its leader, if it’s + * unset). + * + * If so, returns the workspace on which the startup was initiated. + * Returns NULL otherwise. + * + */ +char *startup_workspace_for_window(i3Window *cwindow, xcb_get_property_reply_t *startup_id_reply) { + struct Startup_Sequence *sequence = startup_sequence_get(cwindow, startup_id_reply, false); + if (sequence == NULL) + return NULL; + + /* If the startup sequence's time span has elapsed, delete it. */ + time_t current_time = time(NULL); + if (sequence->delete_at > 0 && current_time > sequence->delete_at) { + DLOG("Deleting expired startup sequence %s\n", sequence->id); + startup_sequence_delete(sequence); + return NULL; + } + return sequence->workspace; } diff --git a/testcases/t/175-startup-notification.t b/testcases/t/175-startup-notification.t index b27a9a70..7160399d 100644 --- a/testcases/t/175-startup-notification.t +++ b/testcases/t/175-startup-notification.t @@ -136,16 +136,49 @@ is_num_children($second_ws, 0, 'still no containers on the second workspace'); is_num_children($first_ws, 2, 'two containers on the first workspace'); ###################################################################### -# 2) open another window after the startup process is completed -# (should be placed on the current workspace) +# verifies that finishing startup doesn't immediately stop windows +# from being placed on the sequence's workspace, but that moving +# the leader actually deletes the startup sequence mapping ###################################################################### complete_startup(); sync_with_i3; -my $otherwin = open_window; +# Startup has completed but the 30-second deletion time hasn't elapsed, +# so this window should still go on the leader's initial workspace. +$win = open_window({ dont_map => 1, client_leader => $leader }); +$win->map; +sync_with_i3; + +is_num_children($first_ws, 3, 'three containers on the first workspace'); + +# Switch to the first workspace and move the focused window to the +# second workspace. +cmd "workspace $first_ws"; +cmd "move workspace $second_ws"; + is_num_children($second_ws, 1, 'one container on the second workspace'); +# Create and switch to a new workspace, just to be safe. +my $third_ws = fresh_workspace; + +# Moving the window between workspaces should have immediately +# removed the startup workspace mapping. New windows with that +# leader should be created on the current workspace. +$win = open_window({ dont_map => 1, client_leader => $leader }); +$win->map; +sync_with_i3; + +is_num_children($third_ws, 1, 'one container on the third workspace'); + +###################################################################### +# 2) open another window after the startup process is completed +# (should be placed on the current workspace) +###################################################################### + +my $otherwin = open_window; +is_num_children($third_ws, 2, 'two containers on the third workspace'); + ###################################################################### # 3) test that the --no-startup-id flag for exec leads to no DESKTOP_STARTUP_ID # environment variable. -- 2.39.5