feat(xworkspaces): Support occupied workspaces (#882)

A workspace is occupied if it is not active and there is at least one window managed by the WM (`_NET_CLIENT_LIST`) that has set `_NET_WM_DESKTOP` to that workspace.

The behavior when `_NET_WM_DESKTOP` is not set is not yet clear but this is unlikely to happen since most WMs will position windows on some desktop.

Closes #874
Fixes #1444
Fixes #1033 

* Set Desktop OCCUPIED if a window moves there

This covers more of an edge-case. I did this first by accident, it might
vanish later on.

* Replace tracking change of WS with currently used WS

* Untrack occupied workspaces

* Track windows and their desktops in pairs

* Match type of occupied_desktops with current_desktop

Because the index needs to be matched later on, type mismatches would be non-ideal.

* Recreate the occupied desktops everytime and remove duplicates

* Readd support for moving windows to other desktops

* Use less characters to empty the vector

* Rename variable storing the desktops

* Recount windows on every occasion

This alone simplifies the management and the lookup for occupation of a
workspace

* Keep track of number of windows in every workspace

* Add debugging output that shall be removed before merging

* Remove obsolete TODO

* m_client_list should always be diff'd, since the desktop may change

Therefore we update the desktop-count tally every time the client_list
changes. It may just be a desktop-change without a change of
clients.size()...

* Add more logging-spam to understand window/desktop lifecycle

* Lock event-handler to serialize handling of events

* Fix occupied workspace counting and change to bool array

Also, performance improvements when diffing new and old client lists

* Fix crash when all clients are removed

* Conform to linter and styleguide

* Shorten conditional as it is standard enough

Since this only guards against 0-divisions, it can be shortened
without risking too much confusion down the road.

* Guard against multiple threads accessing and modifying data

Fixes #1444

Modification of internal data happens through the handle-method, while
the build-method tries to access the data structures for display. Since
some modifications clear e.g. the m_viewports, references may become
invalid between looping over them an accessing them.

The mutex should guard against this simultanuous access.

* Do not 'adopt_lock', because calls come from very different threads

To my understanding, adopt_lock has some dependency on the mutex-ownership. Since
the lock is once called from the inside (in handle) and once from the outside (in
build), there might be a problem. After brief testing, the segfaults happened fewer
times.

See #1444

* Also listen to _NET_WM_DESKTOP

In order to move a window from one desktop to another, it is sufficient
to set the desktop-property of that window. xmonad fires a lot of events
in the case of moving a window, herbstluftwm only updates the
_NET_WM_DESKTOP-atom of the window.

This change reloads the clientlist in order to correctly set the
desktop state "occupied".

* Describe need and use of mutex

It might be possible to relieve the guard in xworkspaces_module::handle,
but I am unsure about this. Since xmonad emits a lot of events on almost
every minor change, I would let the guard keep its post, avoiding
race-conditions in event-handling.

* Give temporary variables better names

* Clarify purpose of loop

About 80% of this comment are taken from
https://github.com/jaagr/polybar/pull/882#discussion_r255317363

* Remove merge-remainder

* Use a simpler method to list occupied desktops.

Co-authored-by: Jérôme Boulmier <jerome.boulmier@outlook.fr>

* Document m_clients field

include/modules/xworkspaces.hpp

@@ -1,6 +1,8 @@
 #pragma once
 
 #include <bitset>
+#include <mutex>
+#include <set>
 
 #include "components/config.hpp"
 #include "components/types.hpp"
@@ -69,9 +71,10 @@ namespace modules {
     void set_desktop_urgent(xcb_window_t window);
 
     bool input(string&& cmd);
-    vector<string> get_desktop_names();
 
    private:
+    static vector<string> get_desktop_names();
+
     static constexpr const char* DEFAULT_ICON{"icon-default"};
     static constexpr const char* DEFAULT_LABEL_STATE{"%icon% %name%"};
     static constexpr const char* DEFAULT_LABEL_MONITOR{"%name%"};
@@ -94,7 +97,10 @@ namespace modules {
     unsigned int m_current_desktop;
     string m_current_desktop_name;
 
-    vector<xcb_window_t> m_clientlist;
+    /**
+     * Maps an xcb window to its desktop number
+     */
+    map<xcb_window_t, unsigned int> m_clients;
     vector<unique_ptr<viewport>> m_viewports;
     map<desktop_state, label_t> m_labels;
     label_t m_monitorlabel;
@@ -104,8 +110,12 @@ namespace modules {
     bool m_scroll{true};
     size_t m_index{0};
 
+    // The following mutex is here to protect the data of this modules.
+    // This can't be achieved using m_buildlock since we "CRTP override" get_output().
+    mutable mutex m_workspace_mutex;
+
     event_timer m_timer{0L, 25L};
   };
-}
+}  // namespace modules
 
 POLYBAR_NS_END

src/modules/xworkspaces.cpp

@@ -1,9 +1,10 @@
+#include "modules/xworkspaces.hpp"
+
 #include <algorithm>
 #include <utility>
 
 #include "drawtypes/iconset.hpp"
 #include "drawtypes/label.hpp"
-#include "modules/xworkspaces.hpp"
 #include "utils/factory.hpp"
 #include "utils/math.hpp"
 #include "x11/atoms.hpp"
@@ -17,7 +18,7 @@ namespace {
   inline bool operator==(const position& a, const position& b) {
     return a.x + a.y == b.x + b.y;
   }
-}
+}  // namespace
 
 namespace modules {
   template class module<xworkspaces_module>;
@@ -86,21 +87,25 @@ namespace modules {
     m_current_desktop_name = m_desktop_names[m_current_desktop];
 
     rebuild_desktops();
-    rebuild_desktop_states();
 
     // Get _NET_CLIENT_LIST
     rebuild_clientlist();
+    rebuild_desktop_states();
   }
 
   /**
    * Handler for XCB_PROPERTY_NOTIFY events
    */
   void xworkspaces_module::handle(const evt::property_notify& evt) {
-    if (evt->atom == m_ewmh->_NET_CLIENT_LIST) {
+    std::lock_guard<std::mutex> lock(m_workspace_mutex);
+
+    if (evt->atom == m_ewmh->_NET_CLIENT_LIST || evt->atom == m_ewmh->_NET_WM_DESKTOP) {
       rebuild_clientlist();
+      rebuild_desktop_states();
     } else if (evt->atom == m_ewmh->_NET_DESKTOP_NAMES || evt->atom == m_ewmh->_NET_NUMBER_OF_DESKTOPS) {
       m_desktop_names = get_desktop_names();
       rebuild_desktops();
+      rebuild_clientlist();
       rebuild_desktop_states();
     } else if (evt->atom == m_ewmh->_NET_CURRENT_DESKTOP) {
       m_current_desktop = ewmh_util::get_current_desktop();
@@ -123,28 +128,21 @@ namespace modules {
    * Rebuild the list of managed clients
    */
   void xworkspaces_module::rebuild_clientlist() {
-    vector<xcb_window_t> clients = ewmh_util::get_client_list();
+    vector<xcb_window_t> newclients = ewmh_util::get_client_list();
-    vector<xcb_window_t> diff;
+    std::sort(newclients.begin(), newclients.end());
-    std::sort(clients.begin(), clients.end());
-    std::sort(m_clientlist.begin(), m_clientlist.end());
 
-    if (m_clientlist.size() > clients.size()) {
+    for (auto&& client : newclients) {
-      std::set_difference(
+      if (m_clients.count(client) == 0) {
-          m_clientlist.begin(), m_clientlist.end(), clients.begin(), clients.end(), back_inserter(diff));
+        // new client: listen for changes (wm_hint or desktop)
-      for (auto&& win : diff) {
+        m_connection.ensure_event_mask(client, XCB_EVENT_MASK_PROPERTY_CHANGE);
-        // untrack window
-        m_clientlist.erase(std::remove(m_clientlist.begin(), m_clientlist.end(), win), m_clientlist.end());
-      }
-    } else {
-      std::set_difference(
-          clients.begin(), clients.end(), m_clientlist.begin(), m_clientlist.end(), back_inserter(diff));
-      for (auto&& win : diff) {
-        // listen for wm_hint (urgency) changes
-        m_connection.ensure_event_mask(win, XCB_EVENT_MASK_PROPERTY_CHANGE);
-        // track window
-        m_clientlist.emplace_back(win);
       }
     }
+
+    // rebuild entire mapping of clients to desktops
+    m_clients.clear();
+    for (auto&& client : newclients) {
+      m_clients[client] = ewmh_util::get_desktop_from_window(client);
+    }
   }
 
   /**
@@ -209,13 +207,20 @@ namespace modules {
    * Update active state of current desktops
    */
   void xworkspaces_module::rebuild_desktop_states() {
+    std::set<unsigned int> occupied_desks;
+    for (auto&& c : m_clients) {
+      occupied_desks.insert(c.second);
+    }
+
     for (auto&& v : m_viewports) {
       for (auto&& d : v->desktops) {
         if (m_desktop_names[d->index] == m_current_desktop_name) {
           d->state = desktop_state::ACTIVE;
+        } else if (occupied_desks.count(d->index) > 0) {
+          d->state = desktop_state::OCCUPIED;
         } else {
           d->state = desktop_state::EMPTY;
-        } 
+        }
 
         d->label = m_labels.at(d->state)->clone();
         d->label->reset_tokens();
@@ -226,14 +231,13 @@ namespace modules {
     }
   }
 
-  vector<string> xworkspaces_module::get_desktop_names(){
+  vector<string> xworkspaces_module::get_desktop_names() {
     vector<string> names = ewmh_util::get_desktop_names();
     unsigned int desktops_number = ewmh_util::get_number_of_desktops();
-    if(desktops_number == names.size()) {
+    if (desktops_number == names.size()) {
       return names;
-    }
+    } else if (desktops_number < names.size()) {
-    else if(desktops_number < names.size()) {
+      names.erase(names.begin() + desktops_number, names.end());
-      names.erase(names.begin()+desktops_number, names.end());
       return names;
     }
     for (unsigned int i = names.size(); i < desktops_number + 1; i++) {
@@ -247,7 +251,7 @@ namespace modules {
    */
   void xworkspaces_module::set_desktop_urgent(xcb_window_t window) {
     auto desk = ewmh_util::get_desktop_from_window(window);
-    if(desk == m_current_desktop)
+    if (desk == m_current_desktop)
       // ignore if current desktop is urgent
       return;
     for (auto&& v : m_viewports) {
@@ -264,7 +268,6 @@ namespace modules {
         }
       }
     }
-
   }
 
   /**
@@ -276,6 +279,8 @@ namespace modules {
    * Generate module output
    */
   string xworkspaces_module::get_output() {
+    std::unique_lock<std::mutex> lock(m_workspace_mutex);
+
     // Get the module output early so that
     // the format prefix/suffix also gets wrapped
     // with the cmd handlers
@@ -335,6 +340,8 @@ namespace modules {
    * Handle user input event
    */
   bool xworkspaces_module::input(string&& cmd) {
+    std::lock_guard<std::mutex> lock(m_workspace_mutex);
+
     size_t len{strlen(EVENT_PREFIX)};
     if (cmd.compare(0, len, EVENT_PREFIX) != 0) {
       return false;
@@ -372,6 +379,6 @@ namespace modules {
 
     return true;
   }
-}
+}  // namespace modules
 
 POLYBAR_NS_END