tray: Store clients in unique_ptr

include/x11/tray_client.hpp

@@ -17,14 +17,11 @@ POLYBAR_NS
 // fwd declarations
 class connection;
 
-class tray_client : public non_copyable_mixin {
+class tray_client : public non_copyable_mixin, public non_movable_mixin {
  public:
   explicit tray_client(const logger& log, connection& conn, xcb_window_t tray, xcb_window_t win, size s);
   ~tray_client();
 
-  tray_client(tray_client&&);
-  tray_client& operator=(tray_client&&);
-
   string name() const;
 
   unsigned int width() const;

include/x11/tray_manager.hpp

@@ -120,6 +120,7 @@ class tray_manager : public xpp::event::sink<evt::expose, evt::client_message, e
   tray_client* find_client(const xcb_window_t& win);
   void remove_client(const tray_client& client);
   void remove_client(xcb_window_t win);
+  void clean_clients();
   bool change_visibility(bool visible);
 
   void handle(const evt::expose& evt) override;
@@ -141,7 +142,7 @@ class tray_manager : public xpp::event::sink<evt::expose, evt::client_message, e
   connection& m_connection;
   signal_emitter& m_sig;
   const logger& m_log;
-  vector<tray_client> m_clients;
+  vector<unique_ptr<tray_client>> m_clients;
 
   tray_settings m_opts{};
   const bar_settings& m_bar_opts;

src/components/bar.cpp

@@ -840,6 +840,7 @@ void bar::handle(const evt::button_press& evt) {
  */
 void bar::handle(const evt::expose& evt) {
   if (evt->window == m_opts.x_data.window && evt->count == 0) {
+    // TODO
     // if (m_tray->running()) {
     //   broadcast_visibility();
     // }

src/x11/tray_client.cpp

@@ -69,31 +69,6 @@ tray_client::~tray_client() {
   }
 }
 
-tray_client::tray_client(tray_client&& c) : m_log(c.m_log), m_connection(c.m_connection) {
-  std::swap(m_name, c.m_name);
-  std::swap(m_wrapper, c.m_wrapper);
-  std::swap(m_client, c.m_client);
-  std::swap(m_xembed_supported, c.m_xembed_supported);
-  std::swap(m_xembed, c.m_xembed);
-  std::swap(m_mapped, c.m_mapped);
-  std::swap(m_hidden, c.m_hidden);
-  std::swap(m_size, c.m_size);
-}
-
-tray_client& tray_client::operator=(tray_client&& c) {
-  m_log = c.m_log;
-  m_connection = c.m_connection;
-  std::swap(m_name, c.m_name);
-  std::swap(m_wrapper, c.m_wrapper);
-  std::swap(m_client, c.m_client);
-  std::swap(m_xembed_supported, c.m_xembed_supported);
-  std::swap(m_xembed, c.m_xembed);
-  std::swap(m_mapped, c.m_mapped);
-  std::swap(m_hidden, c.m_hidden);
-  std::swap(m_size, c.m_size);
-  return *this;
-}
-
 string tray_client::name() const {
   return "tray_client(" + m_connection.id(m_client) + ", " + m_name + ")";
 }

src/x11/tray_manager.cpp

@@ -270,18 +270,27 @@ void tray_manager::reconfigure_clients() {
 
   int x = calculate_x() + m_opts.spacing;
 
-  for (auto it = m_clients.rbegin(); it != m_clients.rend(); it++) {
+  bool has_error = false;
+
+  for (auto& client : m_clients) {
     try {
-      it->ensure_state();
+      client->ensure_state();
-      // TODO skip if the client isn't mapped
+
-      it->reconfigure(x, calculate_client_y());
+      if (client->mapped()) {
+        client->reconfigure(x, calculate_client_y());
+      }
 
       x += m_opts.client_size.w + m_opts.spacing;
     } catch (const xpp::x::error::window& err) {
-      m_log.err("Failed to reconfigure %s, removing ... (%s)", it->name(), err.what());
+      m_log.err("Failed to reconfigure %s, removing ... (%s)", client->name(), err.what());
-      remove_client(*it);
+      client.reset();
+      has_error = true;
     }
   }
+
+  if (has_error) {
+    clean_clients();
+  }
 }
 
 /**
@@ -296,11 +305,11 @@ void tray_manager::refresh_window() {
 
   for (auto& client : m_clients) {
     try {
-      if (client.mapped()) {
+      if (client->mapped()) {
-        client.clear_window();
+        client->clear_window();
       }
     } catch (const std::exception& e) {
-      m_log.err("tray: Failed to clear %s (%s)", client.name(), e.what());
+      m_log.err("tray: Failed to clear %s (%s)", client->name(), e.what());
     }
   }
 
@@ -434,24 +443,24 @@ void tray_manager::process_docking_request(xcb_window_t win) {
   m_log.info("tray: Processing docking request from '%s' (%s)", ewmh_util::get_wm_name(win), m_connection.id(win));
 
   try {
-    tray_client client(m_log, m_connection, m_opts.selection_owner, win, m_opts.client_size);
+    auto client = make_unique<tray_client>(m_log, m_connection, m_opts.selection_owner, win, m_opts.client_size);
 
     try {
-      client.query_xembed();
+      client->query_xembed();
     } catch (const xpp::x::error::window& err) {
-      m_log.err("Failed to query _XEMBED_INFO, removing %s ... (%s)", client.name(), err.what());
+      m_log.err("Failed to query _XEMBED_INFO, removing %s ... (%s)", client->name(), err.what());
       return;
     }
 
-    client.update_client_attributes();
+    client->update_client_attributes();
 
-    client.reparent();
+    client->reparent();
 
-    client.add_to_save_set();
+    client->add_to_save_set();
 
-    client.notify_xembed();
+    client->notify_xembed();
 
-    client.ensure_state();
+    client->ensure_state();
 
     m_clients.emplace_back(std::move(client));
   } catch (const std::exception& err) {
@@ -475,7 +484,7 @@ unsigned tray_manager::calculate_w() const {
   unsigned width = m_opts.spacing;
   unsigned count{0};
   for (auto& client : m_clients) {
-    if (client.mapped()) {
+    if (client->mapped()) {
       count++;
       width += m_opts.spacing + m_opts.client_size.w;
     }
@@ -504,12 +513,12 @@ bool tray_manager::is_embedded(const xcb_window_t& win) {
  */
 tray_client* tray_manager::find_client(const xcb_window_t& win) {
   auto client = std::find_if(m_clients.begin(), m_clients.end(),
-      [win](const auto& client) { return client.match(win) || client.embedder() == win; });
+      [win](const auto& client) { return client->match(win) || client->embedder() == win; });
 
   if (client == m_clients.end()) {
     return nullptr;
   } else {
-    return &(*client);
+    return client->get();
   }
 }
 
@@ -526,13 +535,25 @@ void tray_manager::remove_client(const tray_client& client) {
 void tray_manager::remove_client(xcb_window_t win) {
   auto old_size = m_clients.size();
   m_clients.erase(
-      std::remove_if(m_clients.begin(), m_clients.end(), [win](const auto& client) { return client.match(win); }));
+      std::remove_if(m_clients.begin(), m_clients.end(), [win](const auto& client) { return client->match(win); }));
 
   if (old_size != m_clients.size()) {
     tray_manager::reconfigure();
   }
 }
 
+/**
+ * Remove all null pointers from client list.
+ *
+ * Removing clients is often done in two steps:
+ * 1. When removing a client during iteration, the unique_ptr is reset.
+ * 2. Afterwards all null pointers are removed from the list.
+ */
+void tray_manager::clean_clients() {
+  m_clients.erase(
+      std::remove_if(m_clients.begin(), m_clients.end(), [](const auto& client) { return client.get() == nullptr; }));
+}
+
 bool tray_manager::change_visibility(bool visible) {
   if (!is_active() || m_hidden == !visible) {
     return false;
@@ -543,8 +564,8 @@ bool tray_manager::change_visibility(bool visible) {
   m_hidden = !visible;
 
   for (auto& client : m_clients) {
-    client.hidden(m_hidden);
+    client->hidden(m_hidden);
-    client.ensure_state();
+    client->ensure_state();
   }
 
   if (!m_hidden) {