Properly close disabled outputs restored during a restart. (#2337)

If an output is disabled during a restart, for example because a binding
such as

    bindsym $mod+Shift+r exec "xrandr --auto", restart

is used, it can happen that we first write the layout to disk and only
then receive the RandR change events. This leads to a situation where
the restored tree will contain these outputs, but the restarted i3
process will not receive the RandR events, thus the internal output in i3
is marked disabled.

This patch finds these cases after a restart and force-disables the
affected outputs.

fixes #2326

include/randr.h

@@ -60,6 +60,12 @@ void init_ws_for_output(Output *output, Con *content);
  */
 void randr_query_outputs(void);
 
+/**
+ * Disables the output and moves its content.
+ *
+ */
+void randr_disable_output(Output *output);
+
 /**
  * Returns the first output which is active.
  *

src/main.c

@@ -623,7 +623,7 @@ int main(int argc, char *argv[]) {
     grab_all_keys(conn);
 
     bool needs_tree_init = true;
-    if (layout_path) {
+    if (layout_path != NULL) {
         LOG("Trying to restore the layout from \"%s\".\n", layout_path);
         needs_tree_init = !tree_restore(layout_path, greply);
         if (delete_layout_path) {
@@ -633,7 +633,6 @@ int main(int argc, char *argv[]) {
              * sockets) left. */
             rmdir(dir);
         }
-        free(layout_path);
     }
     if (needs_tree_init)
         tree_init(greply);
@@ -658,6 +657,33 @@ int main(int argc, char *argv[]) {
         randr_init(&randr_base);
     }
 
+    /* We need to force disabling outputs which have been loaded from the
+     * layout file but are no longer active. This can happen if the output has
+     * been disabled in the short time between writing the restart layout file
+     * and restarting i3. See #2326. */
+    if (layout_path != NULL && randr_base > -1) {
+        Con *con;
+        TAILQ_FOREACH(con, &(croot->nodes_head), nodes) {
+            Output *output;
+            TAILQ_FOREACH(output, &outputs, outputs) {
+                if (output->active || strcmp(con->name, output->name) != 0)
+                    continue;
+
+                /* This will correctly correlate the output with its content
+                 * container. We need to make the connection to properly
+                 * disable the output. */
+                if (output->con == NULL) {
+                    output_init_con(output);
+                    output->changed = false;
+                }
+
+                output->to_be_disabled = true;
+                randr_disable_output(output);
+            }
+        }
+    }
+    FREE(layout_path);
+
     scratchpad_fix_resolution();
 
     xcb_query_pointer_reply_t *pointerreply;

src/randr.c

@@ -622,7 +622,7 @@ static void handle_output(xcb_connection_t *conn, xcb_randr_output_t id,
  *
  */
 void randr_query_outputs(void) {
-    Output *output, *other, *first;
+    Output *output, *other;
     xcb_randr_get_output_primary_cookie_t pcookie;
     xcb_randr_get_screen_resources_current_cookie_t rcookie;
 
@@ -733,84 +733,7 @@ void randr_query_outputs(void) {
      * because the user disabled them or because they are clones) */
     TAILQ_FOREACH(output, &outputs, outputs) {
         if (output->to_be_disabled) {
-            output->active = false;
+            randr_disable_output(output);
-            DLOG("Output %s disabled, re-assigning workspaces/docks\n", output->name);
-
-            first = get_first_output();
-
-            /* TODO: refactor the following code into a nice function. maybe
-             * use an on_destroy callback which is implement differently for
-             * different container types (CT_CONTENT vs. CT_DOCKAREA)? */
-            Con *first_content = output_get_content(first->con);
-
-            if (output->con != NULL) {
-                /* We need to move the workspaces from the disappearing output to the first output */
-                /* 1: Get the con to focus next, if the disappearing ws is focused */
-                Con *next = NULL;
-                if (TAILQ_FIRST(&(croot->focus_head)) == output->con) {
-                    DLOG("This output (%p) was focused! Getting next\n", output->con);
-                    next = focused;
-                    DLOG("next = %p\n", next);
-                }
-
-                /* 2: iterate through workspaces and re-assign them, fixing the coordinates
-                 * of floating containers as we go */
-                Con *current;
-                Con *old_content = output_get_content(output->con);
-                while (!TAILQ_EMPTY(&(old_content->nodes_head))) {
-                    current = TAILQ_FIRST(&(old_content->nodes_head));
-                    if (current != next && TAILQ_EMPTY(&(current->focus_head))) {
-                        /* the workspace is empty and not focused, get rid of it */
-                        DLOG("Getting rid of current = %p / %s (empty, unfocused)\n", current, current->name);
-                        tree_close_internal(current, DONT_KILL_WINDOW, false, false);
-                        continue;
-                    }
-                    DLOG("Detaching current = %p / %s\n", current, current->name);
-                    con_detach(current);
-                    DLOG("Re-attaching current = %p / %s\n", current, current->name);
-                    con_attach(current, first_content, false);
-                    DLOG("Fixing the coordinates of floating containers\n");
-                    Con *floating_con;
-                    TAILQ_FOREACH(floating_con, &(current->floating_head), floating_windows)
-                    floating_fix_coordinates(floating_con, &(output->con->rect), &(first->con->rect));
-                    DLOG("Done, next\n");
-                }
-                DLOG("re-attached all workspaces\n");
-
-                if (next) {
-                    DLOG("now focusing next = %p\n", next);
-                    con_focus(next);
-                    workspace_show(con_get_workspace(next));
-                }
-
-                /* 3: move the dock clients to the first output */
-                Con *child;
-                TAILQ_FOREACH(child, &(output->con->nodes_head), nodes) {
-                    if (child->type != CT_DOCKAREA)
-                        continue;
-                    DLOG("Handling dock con %p\n", child);
-                    Con *dock;
-                    while (!TAILQ_EMPTY(&(child->nodes_head))) {
-                        dock = TAILQ_FIRST(&(child->nodes_head));
-                        Con *nc;
-                        Match *match;
-                        nc = con_for_window(first->con, dock->window, &match);
-                        DLOG("Moving dock client %p to nc %p\n", dock, nc);
-                        con_detach(dock);
-                        DLOG("Re-attaching\n");
-                        con_attach(dock, nc, false);
-                        DLOG("Done\n");
-                    }
-                }
-
-                DLOG("destroying disappearing con %p\n", output->con);
-                tree_close_internal(output->con, DONT_KILL_WINDOW, true, false);
-                DLOG("Done. Should be fine now\n");
-                output->con = NULL;
-            }
-
-            output->to_be_disabled = false;
-            output->changed = false;
         }
 
         if (output->changed) {
@@ -846,6 +769,94 @@ void randr_query_outputs(void) {
     FREE(primary);
 }
 
+/*
+ * Disables the output and moves its content.
+ *
+ */
+void randr_disable_output(Output *output) {
+    assert(output->to_be_disabled);
+
+    output->active = false;
+    DLOG("Output %s disabled, re-assigning workspaces/docks\n", output->name);
+
+    Output *first = get_first_output();
+
+    /* TODO: refactor the following code into a nice function. maybe
+     * use an on_destroy callback which is implement differently for
+     * different container types (CT_CONTENT vs. CT_DOCKAREA)? */
+    Con *first_content = output_get_content(first->con);
+
+    if (output->con != NULL) {
+        /* We need to move the workspaces from the disappearing output to the first output */
+        /* 1: Get the con to focus next, if the disappearing ws is focused */
+        Con *next = NULL;
+        if (TAILQ_FIRST(&(croot->focus_head)) == output->con) {
+            DLOG("This output (%p) was focused! Getting next\n", output->con);
+            next = focused;
+            DLOG("next = %p\n", next);
+        }
+
+        /* 2: iterate through workspaces and re-assign them, fixing the coordinates
+         * of floating containers as we go */
+        Con *current;
+        Con *old_content = output_get_content(output->con);
+        while (!TAILQ_EMPTY(&(old_content->nodes_head))) {
+            current = TAILQ_FIRST(&(old_content->nodes_head));
+            if (current != next && TAILQ_EMPTY(&(current->focus_head))) {
+                /* the workspace is empty and not focused, get rid of it */
+                DLOG("Getting rid of current = %p / %s (empty, unfocused)\n", current, current->name);
+                tree_close_internal(current, DONT_KILL_WINDOW, false, false);
+                continue;
+            }
+            DLOG("Detaching current = %p / %s\n", current, current->name);
+            con_detach(current);
+            DLOG("Re-attaching current = %p / %s\n", current, current->name);
+            con_attach(current, first_content, false);
+            DLOG("Fixing the coordinates of floating containers\n");
+            Con *floating_con;
+            TAILQ_FOREACH(floating_con, &(current->floating_head), floating_windows) {
+                floating_fix_coordinates(floating_con, &(output->con->rect), &(first->con->rect));
+            }
+            DLOG("Done, next\n");
+        }
+        DLOG("re-attached all workspaces\n");
+
+        if (next) {
+            DLOG("now focusing next = %p\n", next);
+            con_focus(next);
+            workspace_show(con_get_workspace(next));
+        }
+
+        /* 3: move the dock clients to the first output */
+        Con *child;
+        TAILQ_FOREACH(child, &(output->con->nodes_head), nodes) {
+            if (child->type != CT_DOCKAREA)
+                continue;
+            DLOG("Handling dock con %p\n", child);
+            Con *dock;
+            while (!TAILQ_EMPTY(&(child->nodes_head))) {
+                dock = TAILQ_FIRST(&(child->nodes_head));
+                Con *nc;
+                Match *match;
+                nc = con_for_window(first->con, dock->window, &match);
+                DLOG("Moving dock client %p to nc %p\n", dock, nc);
+                con_detach(dock);
+                DLOG("Re-attaching\n");
+                con_attach(dock, nc, false);
+                DLOG("Done\n");
+            }
+        }
+
+        DLOG("destroying disappearing con %p\n", output->con);
+        tree_close_internal(output->con, DONT_KILL_WINDOW, true, false);
+        DLOG("Done. Should be fine now\n");
+        output->con = NULL;
+    }
+
+    output->to_be_disabled = false;
+    output->changed = false;
+}
+
 /*
  * We have just established a connection to the X server and need the initial
  * XRandR information to setup workspaces for each screen.